Vectrum
Sign In
CodeyPublic
Like0 likesFork0 forks

Plan: implement skills in six tasks

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Browse Files
Jjonicommitted 1 parent d5ef6afcommit30bb8e2

Showing 2 changed files with 1425 additions and 7 deletions.

+1414−0docs/superpowers/plans/2026-09-17-agents-skills.mdaddedView File
@@ -0,0 +1,1414 @@
1+ # Skills Implementation Plan
2+
3+ > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+ **Goal:** Let the agent invoke instruction sets from `~/.agents/skills` and `<repo>/.agents/skills` on its own, and let the user force one with `/slug`.
6+
7+ **Architecture:** A new `internal/skills` package discovers `<root>/<slug>/SKILL.md` files and parses their YAML frontmatter. A new `skill` tool returns a body on request; it is the only thing that loads a body, so agent-invoked and user-invoked paths are the same code. The slug/description listing is appended to the system prompt once at agent construction, keeping the cached prefix constant for the session. The path jail grows read-only extra roots so a skill can read its own bundled files.
8+
9+ **Tech Stack:** Go 1.26, `gopkg.in/yaml.v3` (new), `invopop/jsonschema` (existing, for the tool schema), Bubble Tea (existing, for the TUI).
10+
11+ **Spec:** `docs/superpowers/specs/2026-09-17-agents-skills-design.md`
12+
13+ ## Global Constraints
14+
15+ - Go module is `harness`; import paths are `harness/internal/...`.
16+ - `internal/agent` must never import `internal/ui` or `internal/tui`. It may import `internal/skills`.
17+ - Every tool returns `(string, error)` where the error is text the model reads.
18+ - Slugs match `^[a-z0-9][a-z0-9-]*$`.
19+ - Frontmatter `name` must equal the directory name, or the skill is a problem.
20+ - Skills are readable, never writable: `edit_file`, `write_file` and `bash` get no extra roots.
21+ - Nothing about skills aborts startup. Every discovery failure is a problem, not an error.
22+ - Discovery is startup-only. No hot reload.
23+ - With zero skills discovered: no tool registered, nothing appended to the system prompt, no extra jail roots. The prompt must be byte-identical to today's.
24+ - New exported identifiers get a doc comment, matching the existing house style.
25+ - Run `gofmt -w` on every file you touch before committing.
26+
27+ ---
28+
29+ ### Task 1: The `internal/skills` package
30+
31+ **Files:**
32+ - Create: `internal/skills/skills.go`
33+ - Create: `internal/skills/skills_test.go`
34+ - Modify: `go.mod`, `go.sum`
35+
36+ **Interfaces:**
37+ - Consumes: nothing.
38+ - Produces: `skills.Skill{Name, Description, Body, Dir, Source string}` and `skills.Discover(home, workroot string) ([]Skill, []error)`. Later tasks depend on both. `Source` is the literal string `"personal"` or `"project"`. The returned slice is sorted by `Name`.
39+
40+ - [ ] **Step 1: Add the YAML dependency**
41+
42+ ```bash
43+ cd /home/joni/Work/GO/coding-harness
44+ go get gopkg.in/yaml.v3@v3.0.1
45+ ```
46+
47+ - [ ] **Step 2: Write the failing tests**
48+
49+ Create `internal/skills/skills_test.go`:
50+
51+ ```go
52+ package skills
53+
54+ import (
55+ "os"
56+ "path/filepath"
57+ "strings"
58+ "testing"
59+ )
60+
61+ // write creates <root>/<slug>/SKILL.md with the given contents.
62+ func write(t *testing.T, root, slug, body string) string {
63+ t.Helper()
64+ dir := filepath.Join(root, slug)
65+ if err := os.MkdirAll(dir, 0o755); err != nil {
66+ t.Fatal(err)
67+ }
68+ if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(body), 0o644); err != nil {
69+ t.Fatal(err)
70+ }
71+ return dir
72+ }
73+
74+ const wellFormed = `---
75+ name: commit-helper
76+ description: Use when writing commit messages
77+ ---
78+
79+ Read the last twenty commits for tone.
80+ `
81+
82+ func TestDiscoverWellFormed(t *testing.T) {
83+ home, work := t.TempDir(), t.TempDir()
84+ dir := write(t, filepath.Join(home, ".agents", "skills"), "commit-helper", wellFormed)
85+
86+ found, problems := Discover(home, work)
87+ if len(problems) != 0 {
88+ t.Fatalf("problems: %v", problems)
89+ }
90+ if len(found) != 1 {
91+ t.Fatalf("got %d skills, want 1", len(found))
92+ }
93+ s := found[0]
94+ if s.Name != "commit-helper" {
95+ t.Errorf("Name = %q", s.Name)
96+ }
97+ if s.Description != "Use when writing commit messages" {
98+ t.Errorf("Description = %q", s.Description)
99+ }
100+ if s.Source != "personal" {
101+ t.Errorf("Source = %q, want personal", s.Source)
102+ }
103+ if s.Dir != dir {
104+ t.Errorf("Dir = %q, want %q", s.Dir, dir)
105+ }
106+ if !strings.Contains(s.Body, "last twenty commits") {
107+ t.Errorf("Body = %q", s.Body)
108+ }
109+ if strings.Contains(s.Body, "description:") {
110+ t.Error("Body still contains the frontmatter")
111+ }
112+ }
113+
114+ func TestProjectShadowsPersonal(t *testing.T) {
115+ home, work := t.TempDir(), t.TempDir()
116+ write(t, filepath.Join(home, ".agents", "skills"), "dup", `---
117+ name: dup
118+ description: personal one
119+ ---
120+ personal body
121+ `)
122+ write(t, filepath.Join(work, ".agents", "skills"), "dup", `---
123+ name: dup
124+ description: project one
125+ ---
126+ project body
127+ `)
128+
129+ found, problems := Discover(home, work)
130+ if len(problems) != 0 {
131+ t.Fatalf("problems: %v", problems)
132+ }
133+ if len(found) != 1 {
134+ t.Fatalf("got %d skills, want 1", len(found))
135+ }
136+ if found[0].Description != "project one" || found[0].Source != "project" {
137+ t.Errorf("personal skill was not shadowed: %+v", found[0])
138+ }
139+ }
140+
141+ func TestMalformedSkillsAreProblemsAndDoNotBlockOthers(t *testing.T) {
142+ home, work := t.TempDir(), t.TempDir()
143+ root := filepath.Join(home, ".agents", "skills")
144+ write(t, root, "good", wellFormed2())
145+ write(t, root, "no-frontmatter", "just a body, no frontmatter\n")
146+ write(t, root, "no-description", "---\nname: no-description\n---\nbody\n")
147+ write(t, root, "wrong-name", "---\nname: something-else\ndescription: d\n---\nbody\n")
148+ write(t, root, "Bad_Slug", "---\nname: Bad_Slug\ndescription: d\n---\nbody\n")
149+
150+ found, problems := Discover(home, work)
151+ if len(found) != 1 || found[0].Name != "good" {
152+ t.Fatalf("got %+v, want only the good skill", found)
153+ }
154+ if len(problems) != 4 {
155+ t.Fatalf("got %d problems, want 4: %v", len(problems), problems)
156+ }
157+ // Every problem names the offending path, or it is not actionable.
158+ for _, p := range problems {
159+ if !strings.Contains(p.Error(), root) {
160+ t.Errorf("problem does not name a path: %v", p)
161+ }
162+ }
163+ }
164+
165+ func wellFormed2() string {
166+ return "---\nname: good\ndescription: a good skill\n---\nbody\n"
167+ }
168+
169+ func TestDescriptionScalarStyles(t *testing.T) {
170+ // A folded and a quoted description must parse to the same text as a plain
171+ // one. This is the whole reason for the yaml dependency.
172+ cases := map[string]string{
173+ "plain": "description: Use when writing commit messages",
174+ "quoted": `description: "Use when writing commit messages"`,
175+ "folded": "description: >-\n Use when writing\n commit messages",
176+ }
177+ for slug, line := range cases {
178+ t.Run(slug, func(t *testing.T) {
179+ home, work := t.TempDir(), t.TempDir()
180+ write(t, filepath.Join(home, ".agents", "skills"), slug,
181+ "---\nname: "+slug+"\n"+line+"\n---\nbody\n")
182+
183+ found, problems := Discover(home, work)
184+ if len(problems) != 0 {
185+ t.Fatalf("problems: %v", problems)
186+ }
187+ if len(found) != 1 || found[0].Description != "Use when writing commit messages" {
188+ t.Fatalf("Description = %+v", found)
189+ }
190+ })
191+ }
192+ }
193+
194+ func TestNoRootsIsSilent(t *testing.T) {
195+ found, problems := Discover(t.TempDir(), t.TempDir())
196+ if len(found) != 0 || len(problems) != 0 {
197+ t.Fatalf("found %v, problems %v; want both empty", found, problems)
198+ }
199+ }
200+
201+ func TestNestedSkillIsNotDiscovered(t *testing.T) {
202+ home, work := t.TempDir(), t.TempDir()
203+ write(t, filepath.Join(home, ".agents", "skills", "outer"), "inner", wellFormed2())
204+
205+ found, problems := Discover(home, work)
206+ if len(found) != 0 {
207+ t.Fatalf("found %+v, want none", found)
208+ }
209+ // "outer" has no SKILL.md of its own; that is not a problem, just not a skill.
210+ if len(problems) != 0 {
211+ t.Fatalf("problems: %v", problems)
212+ }
213+ }
214+
215+ func TestSortedByName(t *testing.T) {
216+ home, work := t.TempDir(), t.TempDir()
217+ root := filepath.Join(home, ".agents", "skills")
218+ for _, slug := range []string{"zebra", "alpha", "middle"} {
219+ write(t, root, slug, "---\nname: "+slug+"\ndescription: d\n---\nbody\n")
220+ }
221+ found, _ := Discover(home, work)
222+ var got []string
223+ for _, s := range found {
224+ got = append(got, s.Name)
225+ }
226+ if strings.Join(got, ",") != "alpha,middle,zebra" {
227+ t.Errorf("order = %v", got)
228+ }
229+ }
230+ ```
231+
232+ - [ ] **Step 3: Run the tests to verify they fail**
233+
234+ Run: `go test ./internal/skills/`
235+ Expected: FAIL — `undefined: Discover`, `undefined: Skill`.
236+
237+ - [ ] **Step 4: Write the implementation**
238+
239+ Create `internal/skills/skills.go`:
240+
241+ ```go
242+ // Package skills discovers instruction sets stored as Markdown on disk.
243+ //
244+ // A skill is a directory containing SKILL.md: YAML frontmatter naming and
245+ // describing it, followed by the instructions. The description is all the
246+ // model sees until the skill is invoked, so it carries the whole triggering
247+ // decision; the body can be any length.
248+ package skills
249+
250+ import (
251+ "fmt"
252+ "os"
253+ "path/filepath"
254+ "regexp"
255+ "sort"
256+ "strings"
257+
258+ "gopkg.in/yaml.v3"
259+ )
260+
261+ // Dirname is the directory, under the home directory and under the working
262+ // root, that skills are read from.
263+ const Dirname = ".agents/skills"
264+
265+ // filename is the one file a skill directory must contain.
266+ const filename = "SKILL.md"
267+
268+ // slugPattern is what a directory name must match to be a skill. The slug
269+ // becomes a slash command and a JSON enum value, and neither wants spaces.
270+ var slugPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`)
271+
272+ // Skill is one discovered instruction set.
273+ type Skill struct {
274+ Name string // slug, taken from the directory name
275+ Description string // the one line the model sees before invoking
276+ Body string // SKILL.md with the frontmatter removed
277+ Dir string // absolute path to the skill directory
278+ Source string // "personal" or "project"
279+ }
280+
281+ // frontmatter is the subset of SKILL.md's YAML header this harness reads.
282+ // Unknown keys are ignored rather than rejected, so a skill written for
283+ // another harness still loads here.
284+ type frontmatter struct {
285+ Name string `yaml:"name"`
286+ Description string `yaml:"description"`
287+ }
288+
289+ // Discover scans the personal and project roots, project shadowing personal on
290+ // a slug collision, and returns the skills sorted by name.
291+ //
292+ // Every failure is a problem rather than an error: a harness that will not open
293+ // because of a stray file in the home directory is worse than one that runs
294+ // without the skill it was going to load. A root that does not exist is not
295+ // even a problem — neither exists on a fresh machine, and that is the normal
296+ // case.
297+ func Discover(home, workroot string) ([]Skill, []error) {
298+ byName := map[string]Skill{}
299+ var problems []error
300+
301+ // Later roots win, so the project root is scanned second.
302+ for _, r := range []struct{ dir, source string }{
303+ {filepath.Join(home, Dirname), "personal"},
304+ {filepath.Join(workroot, Dirname), "project"},
305+ } {
306+ found, probs := scan(r.dir, r.source)
307+ problems = append(problems, probs...)
308+ for _, s := range found {
309+ byName[s.Name] = s
310+ }
311+ }
312+
313+ out := make([]Skill, 0, len(byName))
314+ for _, s := range byName {
315+ out = append(out, s)
316+ }
317+ sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name })
318+ return out, problems
319+ }
320+
321+ // scan reads one root, one level deep.
322+ func scan(root, source string) ([]Skill, []error) {
323+ entries, err := os.ReadDir(root)
324+ if os.IsNotExist(err) {
325+ return nil, nil
326+ }
327+ if err != nil {
328+ return nil, []error{fmt.Errorf("%s: %w", root, err)}
329+ }
330+
331+ var found []Skill
332+ var problems []error
333+ for _, e := range entries {
334+ if !e.IsDir() {
335+ continue
336+ }
337+ dir := filepath.Join(root, e.Name())
338+ path := filepath.Join(dir, filename)
339+ if _, err := os.Stat(path); err != nil {
340+ // A directory without a SKILL.md is not a skill, and not a
341+ // complaint either: it may be anything at all.
342+ continue
343+ }
344+ s, err := load(dir, e.Name(), source)
345+ if err != nil {
346+ problems = append(problems, err)
347+ continue
348+ }
349+ found = append(found, s)
350+ }
351+ return found, problems
352+ }
353+
354+ // load parses one SKILL.md. slug is the directory name, which is the authority
355+ // for the skill's name.
356+ func load(dir, slug, source string) (Skill, error) {
357+ path := filepath.Join(dir, filename)
358+ if !slugPattern.MatchString(slug) {
359+ return Skill{}, fmt.Errorf("%s: directory name %q is not a valid skill name; use lower-case letters, digits and hyphens", path, slug)
360+ }
361+ raw, err := os.ReadFile(path)
362+ if err != nil {
363+ return Skill{}, fmt.Errorf("%s: %w", path, err)
364+ }
365+ head, body, err := split(string(raw))
366+ if err != nil {
367+ return Skill{}, fmt.Errorf("%s: %w", path, err)
368+ }
369+ var fm frontmatter
370+ if err := yaml.Unmarshal([]byte(head), &fm); err != nil {
371+ return Skill{}, fmt.Errorf("%s: frontmatter is not valid YAML: %w", path, err)
372+ }
373+ switch {
374+ case strings.TrimSpace(fm.Description) == "":
375+ return Skill{}, fmt.Errorf("%s: frontmatter has no description; it is the only thing the model sees before invoking the skill", path)
376+ case strings.TrimSpace(fm.Name) == "":
377+ return Skill{}, fmt.Errorf("%s: frontmatter has no name; it must be %q, matching the directory", path, slug)
378+ case strings.TrimSpace(fm.Name) != slug:
379+ return Skill{}, fmt.Errorf("%s: frontmatter name is %q but the directory is %q; they must agree, and the directory wins", path, fm.Name, slug)
380+ }
381+ abs, err := filepath.Abs(dir)
382+ if err != nil {
383+ abs = dir
384+ }
385+ return Skill{
386+ Name: slug,
387+ Description: strings.TrimSpace(fm.Description),
388+ Body: strings.TrimSpace(body),
389+ Dir: abs,
390+ Source: source,
391+ }, nil
392+ }
393+
394+ // split separates the leading --- fenced YAML block from the body.
395+ func split(s string) (head, body string, err error) {
396+ s = strings.ReplaceAll(s, "\r\n", "\n")
397+ if !strings.HasPrefix(s, "---\n") {
398+ return "", "", fmt.Errorf("does not begin with a --- frontmatter block")
399+ }
400+ rest := s[len("---\n"):]
401+ end := strings.Index(rest, "\n---")
402+ if end < 0 {
403+ return "", "", fmt.Errorf("frontmatter block is never closed with ---")
404+ }
405+ head = rest[:end]
406+ body = rest[end+len("\n---"):]
407+ if i := strings.IndexByte(body, '\n'); i >= 0 {
408+ body = body[i+1:]
409+ } else {
410+ body = ""
411+ }
412+ return head, body, nil
413+ }
414+ ```
415+
416+ - [ ] **Step 5: Run the tests to verify they pass**
417+
418+ Run: `go test ./internal/skills/ -v`
419+ Expected: PASS, all cases.
420+
421+ - [ ] **Step 6: Commit**
422+
423+ ```bash
424+ cd /home/joni/Work/GO/coding-harness
425+ gofmt -w internal/skills/
426+ go mod tidy
427+ git add internal/skills go.mod go.sum
428+ git commit -m "skills: discover SKILL.md under ~/.agents/skills and .agents/skills"
429+ ```
430+
431+ ---
432+
433+ ### Task 2: Read-only extra roots in the path jail
434+
435+ **Files:**
436+ - Modify: `internal/tools/read.go` (`jail` at line 141, `resolve` at line 114, `NewRead` at line 40)
437+ - Modify: `internal/tools/glob.go:45`
438+ - Modify: `internal/tools/grep.go:74`
439+ - Test: `internal/tools/jail_test.go`
440+
441+ **Interfaces:**
442+ - Consumes: nothing from Task 1.
443+ - Produces: `jailWith(root string, extra []string, path string) (string, error)` and variadic constructors `NewRead(root string, track ReadTracker, extra ...string)`, `NewGlob(root string, extra ...string)`, `NewGrep(root string, extra ...string)`. Task 6 passes skill directories to all three.
444+
445+ The variadic tail is deliberate: every existing call site — `cmd/harness/main.go:104-106`, `evals/runner.go:192`, and the tool tests — keeps compiling unchanged.
446+
447+ - [ ] **Step 1: Write the failing tests**
448+
449+ Append to `internal/tools/jail_test.go`:
450+
451+ ```go
452+ // A skill lives outside the working root. The read tools reach into it; the
453+ // write tools must not.
454+ func TestExtraRootsAreReadableNotWritable(t *testing.T) {
455+ root := t.TempDir()
456+ skill := t.TempDir()
457+ ref := filepath.Join(skill, "references.md")
458+ if err := os.WriteFile(ref, []byte("conventions\n"), 0o644); err != nil {
459+ t.Fatal(err)
460+ }
461+
462+ read := NewRead(root, nil, skill)
463+ out, err := read.Execute(context.Background(), jsonArgs(t, map[string]any{"path": ref}))
464+ if err != nil {
465+ t.Fatalf("read_file refused an extra root: %v", err)
466+ }
467+ if !strings.Contains(out, "conventions") {
468+ t.Errorf("read_file returned %q", out)
469+ }
470+
471+ write := NewWrite(root, nil)
472+ if _, err := write.Execute(context.Background(), jsonArgs(t, map[string]any{
473+ "path": ref, "content": "rewritten",
474+ })); err == nil {
475+ t.Error("write_file was allowed into a skill directory")
476+ }
477+ }
478+
479+ func TestExtraRootsDoNotWidenTheJail(t *testing.T) {
480+ root := t.TempDir()
481+ skill := t.TempDir()
482+ outside := filepath.Join(t.TempDir(), "secret.txt")
483+ if err := os.WriteFile(outside, []byte("secret\n"), 0o644); err != nil {
484+ t.Fatal(err)
485+ }
486+
487+ read := NewRead(root, nil, skill)
488+ if _, err := read.Execute(context.Background(), jsonArgs(t, map[string]any{"path": outside})); err == nil {
489+ t.Error("a path under neither the root nor an extra root was allowed")
490+ }
491+ }
492+
493+ func TestExtraRootSymlinkCannotStepSideways(t *testing.T) {
494+ root := t.TempDir()
495+ skill := t.TempDir()
496+ outside := t.TempDir()
497+ if err := os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret\n"), 0o644); err != nil {
498+ t.Fatal(err)
499+ }
500+ link := filepath.Join(skill, "escape")
501+ if err := os.Symlink(outside, link); err != nil {
502+ t.Skipf("symlinks unavailable: %v", err)
503+ }
504+
505+ read := NewRead(root, nil, skill)
506+ if _, err := read.Execute(context.Background(), jsonArgs(t, map[string]any{
507+ "path": filepath.Join(link, "secret.txt"),
508+ })); err == nil {
509+ t.Error("a symlink out of a skill directory was followed")
510+ }
511+ }
512+
513+ func TestGlobAndGrepAcceptExtraRoots(t *testing.T) {
514+ root := t.TempDir()
515+ skill := t.TempDir()
516+ if err := os.WriteFile(filepath.Join(skill, "notes.md"), []byte("needle\n"), 0o644); err != nil {
517+ t.Fatal(err)
518+ }
519+
520+ g := NewGlob(root, skill)
521+ if _, err := g.Execute(context.Background(), jsonArgs(t, map[string]any{
522+ "pattern": "*.md", "path": skill,
523+ })); err != nil {
524+ t.Errorf("glob refused an extra root: %v", err)
525+ }
526+
527+ gr := NewGrep(root, skill)
528+ out, err := gr.Execute(context.Background(), jsonArgs(t, map[string]any{
529+ "pattern": "needle", "path": skill,
530+ }))
531+ if err != nil {
532+ t.Errorf("grep refused an extra root: %v", err)
533+ }
534+ if !strings.Contains(out, "needle") {
535+ t.Errorf("grep returned %q", out)
536+ }
537+ }
538+ ```
539+
540+ If `jsonArgs` does not already exist in the `tools` test files, add it to `jail_test.go`:
541+
542+ ```go
543+ // jsonArgs marshals a tool's arguments, failing the test rather than the tool.
544+ func jsonArgs(t *testing.T, m map[string]any) json.RawMessage {
545+ t.Helper()
546+ b, err := json.Marshal(m)
547+ if err != nil {
548+ t.Fatal(err)
549+ }
550+ return b
551+ }
552+ ```
553+
554+ Check first — run `grep -rn "func jsonArgs" internal/tools/` and reuse whatever is there, matching its existing signature.
555+
556+ - [ ] **Step 2: Run the tests to verify they fail**
557+
558+ Run: `go test ./internal/tools/ -run 'ExtraRoot|GlobAndGrep' -v`
559+ Expected: FAIL to compile — `too many arguments in call to NewRead`.
560+
561+ - [ ] **Step 3: Replace `jail` with `jailWith` in `internal/tools/read.go`**
562+
563+ Replace the existing `jail` function (line 141) with the two below, keeping the original doc comment's intent:
564+
565+ ```go
566+ // jail refuses paths outside the working root. Symlinks are resolved first, on
567+ // the existing part of the path, so a link pointing out of the tree cannot be
568+ // used to step around the check. It is cheap and catches a lot; it is not a
569+ // substitute for a sandbox, since bash can still cd anywhere.
570+ func jail(root, path string) (string, error) {
571+ return jailWith(root, nil, path)
572+ }
573+
574+ // jailWith is jail with additional permitted roots, used by the read-only
575+ // tools so a skill can read the files it ships with. Each extra root gets the
576+ // same symlink resolution as the working root, so an extra root cannot be used
577+ // to step sideways into the rest of the filesystem.
578+ func jailWith(root string, extra []string, path string) (string, error) {
579+ if root == "" {
580+ return path, nil
581+ }
582+ // A file being created does not exist yet, so resolve the deepest existing
583+ // ancestor and re-attach the rest.
584+ probe, rest := path, ""
585+ for {
586+ if resolved, err := filepath.EvalSymlinks(probe); err == nil {
587+ path = filepath.Join(resolved, rest)
588+ break
589+ }
590+ parent := filepath.Dir(probe)
591+ if parent == probe {
592+ path = filepath.Clean(path)
593+ break
594+ }
595+ rest = filepath.Join(filepath.Base(probe), rest)
596+ probe = parent
597+ }
598+
599+ realRoot := realpath(root)
600+ if contains(realRoot, path) {
601+ return path, nil
602+ }
603+ for _, e := range extra {
604+ if e != "" && contains(realpath(e), path) {
605+ return path, nil
606+ }
607+ }
608+ if len(extra) > 0 {
609+ return "", fmt.Errorf("%s is outside the working root %s and every skill directory; this harness only touches files under the root", path, realRoot)
610+ }
611+ return "", fmt.Errorf("%s is outside the working root %s; this harness only touches files under the root", path, realRoot)
612+ }
613+
614+ // realpath resolves symlinks, falling back to a lexical clean for a path that
615+ // does not exist.
616+ func realpath(p string) string {
617+ if resolved, err := filepath.EvalSymlinks(p); err == nil {
618+ return resolved
619+ }
620+ return filepath.Clean(p)
621+ }
622+
623+ // contains reports whether path is root or sits beneath it.
624+ func contains(root, path string) bool {
625+ rel, err := filepath.Rel(root, path)
626+ if err != nil {
627+ return false
628+ }
629+ return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
630+ }
631+ ```
632+
633+ - [ ] **Step 4: Thread extra roots through the three read tools**
634+
635+ In `internal/tools/read.go`, change `resolve` and `NewRead`:
636+
637+ ```go
638+ // resolve anchors a relative path to root and refuses anything outside it and
639+ // outside every extra root.
640+ func resolve(root string, extra []string, path string) (string, error) {
641+ if path == "" {
642+ return "", fmt.Errorf("path is required")
643+ }
644+ return jailWith(root, extra, resolveBase(root, path))
645+ }
646+
647+ // NewRead builds the read_file tool. root anchors relative paths; track may be
648+ // nil; extra names additional directories that may be read but never written.
649+ func NewRead(root string, track ReadTracker, extra ...string) Tool {
650+ ```
651+
652+ and inside its closure, line 49 becomes:
653+
654+ ```go
655+ path, err := resolve(root, extra, in.Path)
656+ ```
657+
658+ `internal/tools/edit.go:27` and `internal/tools/write.go:26` call `resolve(root, in.Path)`; change both to `resolve(root, nil, in.Path)`. They get no extra roots — a skill is input, not a work product.
659+
660+ In `internal/tools/glob.go`, change the signature to `func NewGlob(root string, extra ...string) Tool` and line 45 to:
661+
662+ ```go
663+ base, err := jailWith(root, extra, resolveBase(root, in.Path))
664+ ```
665+
666+ In `internal/tools/grep.go`, change the signature to `func NewGrep(root string, extra ...string) Tool` and line 74 to:
667+
668+ ```go
669+ base, err := jailWith(root, extra, resolveBase(root, in.Path))
670+ ```
671+
672+ - [ ] **Step 5: Run the whole tools suite**
673+
674+ Run: `go test ./internal/tools/`
675+ Expected: PASS. The existing jail tests must still pass unchanged — if any fails, the refactor changed behaviour for the no-extra-roots case, which it must not.
676+
677+ - [ ] **Step 6: Verify nothing else broke**
678+
679+ Run: `go build ./... && go test ./...`
680+ Expected: PASS. Existing call sites compile because the new parameters are variadic.
681+
682+ - [ ] **Step 7: Commit**
683+
684+ ```bash
685+ cd /home/joni/Work/GO/coding-harness
686+ gofmt -w internal/tools/
687+ git add internal/tools
688+ git commit -m "tools: read-only extra roots in the path jail"
689+ ```
690+
691+ ---
692+
693+ ### Task 3: The `skill` tool
694+
695+ **Files:**
696+ - Create: `internal/tools/skill.go`
697+ - Create: `internal/tools/skill_test.go`
698+ - Modify: `internal/approval/policy.go:40-42`
699+ - Test: `internal/approval/policy_test.go`
700+
701+ **Interfaces:**
702+ - Consumes: `skills.Skill` and `skills.Discover` from Task 1.
703+ - Produces: `tools.NewSkill(found []skills.Skill) Tool`, registered under the name `skill`. Task 6 calls it. Returns `nil` when `found` is empty, and the caller must not register a nil tool.
704+
705+ - [ ] **Step 1: Write the failing tests**
706+
707+ Create `internal/tools/skill_test.go`:
708+
709+ ```go
710+ package tools
711+
712+ import (
713+ "context"
714+ "encoding/json"
715+ "strings"
716+ "testing"
717+
718+ "harness/internal/skills"
719+ )
720+
721+ func testSkills() []skills.Skill {
722+ return []skills.Skill{
723+ {Name: "alpha", Description: "the first one", Body: "do the alpha thing", Dir: "/skills/alpha", Source: "personal"},
724+ {Name: "beta", Description: "the second one", Body: "do the beta thing", Dir: "/skills/beta", Source: "project"},
725+ }
726+ }
727+
728+ func TestSkillReturnsBodyWithDirectory(t *testing.T) {
729+ tool := NewSkill(testSkills())
730+ out, err := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "alpha"}))
731+ if err != nil {
732+ t.Fatal(err)
733+ }
734+ for _, want := range []string{"Skill: alpha", "Directory: /skills/alpha", "do the alpha thing"} {
735+ if !strings.Contains(out, want) {
736+ t.Errorf("output missing %q:\n%s", want, out)
737+ }
738+ }
739+ }
740+
741+ func TestSkillAppendsArguments(t *testing.T) {
742+ tool := NewSkill(testSkills())
743+ out, err := tool.Execute(context.Background(), jsonArgs(t, map[string]any{
744+ "name": "beta", "args": "fix the flaky test",
745+ }))
746+ if err != nil {
747+ t.Fatal(err)
748+ }
749+ if !strings.Contains(out, "ARGUMENTS: fix the flaky test") {
750+ t.Errorf("arguments missing:\n%s", out)
751+ }
752+ }
753+
754+ func TestSkillOmitsArgumentsLineWhenEmpty(t *testing.T) {
755+ tool := NewSkill(testSkills())
756+ out, _ := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "alpha"}))
757+ if strings.Contains(out, "ARGUMENTS:") {
758+ t.Errorf("empty args produced an ARGUMENTS line:\n%s", out)
759+ }
760+ }
761+
762+ func TestSkillUnknownNameListsTheRealOnes(t *testing.T) {
763+ tool := NewSkill(testSkills())
764+ _, err := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "nope"}))
765+ if err == nil {
766+ t.Fatal("unknown skill was accepted")
767+ }
768+ if !strings.Contains(err.Error(), "alpha") || !strings.Contains(err.Error(), "beta") {
769+ t.Errorf("error does not list the available skills: %v", err)
770+ }
771+ }
772+
773+ func TestSkillSchemaEnumeratesSlugs(t *testing.T) {
774+ tool := NewSkill(testSkills())
775+ var schema struct {
776+ Properties struct {
777+ Name struct {
778+ Enum []string `json:"enum"`
779+ } `json:"name"`
780+ } `json:"properties"`
781+ }
782+ if err := json.Unmarshal(tool.InputSchema(), &schema); err != nil {
783+ t.Fatal(err)
784+ }
785+ if strings.Join(schema.Properties.Name.Enum, ",") != "alpha,beta" {
786+ t.Errorf("enum = %v, want [alpha beta]", schema.Properties.Name.Enum)
787+ }
788+ }
789+
790+ func TestSkillDescriptionListsSkills(t *testing.T) {
791+ tool := NewSkill(testSkills())
792+ d := tool.Description()
793+ if !strings.Contains(d, "alpha: the first one") || !strings.Contains(d, "beta: the second one") {
794+ t.Errorf("tool description does not list the skills:\n%s", d)
795+ }
796+ }
797+
798+ func TestNewSkillWithNoneIsNil(t *testing.T) {
799+ if NewSkill(nil) != nil {
800+ t.Error("NewSkill(nil) must return nil so the caller registers nothing")
801+ }
802+ }
803+
804+ func TestSkillRepeatInvocationReturnsBodyAgain(t *testing.T) {
805+ // Deduping would save tokens and cost the model the ability to re-read
806+ // instructions that compaction dropped.
807+ tool := NewSkill(testSkills())
808+ first, _ := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "alpha"}))
809+ second, _ := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "alpha"}))
810+ if first != second || first == "" {
811+ t.Error("the second invocation did not return the same body")
812+ }
813+ }
814+ ```
815+
816+ Add to `internal/approval/policy_test.go`:
817+
818+ ```go
819+ func TestSkillRunsUnattended(t *testing.T) {
820+ p := New(func(Request) Decision { t.Fatal("skill should not prompt"); return Deny })
821+ if d := p.Decide(Request{Tool: "skill"}); d != Allow {
822+ t.Errorf("skill decided %v, want Allow", d)
823+ }
824+ }
825+ ```
826+
827+ Match the existing helpers in `policy_test.go` — read it first and copy the shape of the neighbouring test for `read_file`, including the exact constructor and `Request` field names.
828+
829+ - [ ] **Step 2: Run the tests to verify they fail**
830+
831+ Run: `go test ./internal/tools/ -run Skill -v && go test ./internal/approval/ -run Skill -v`
832+ Expected: FAIL — `undefined: NewSkill`, and the approval test fails because `skill` is not in the auto-approve set.
833+
834+ - [ ] **Step 3: Write the tool**
835+
836+ Create `internal/tools/skill.go`:
837+
838+ ```go
839+ package tools
840+
841+ import (
842+ "context"
843+ "encoding/json"
844+ "fmt"
845+ "sort"
846+ "strings"
847+
848+ "harness/internal/skills"
849+ )
850+
851+ // SkillArgs is the skill input. The name is constrained to the discovered
852+ // slugs by an enum patched into the schema, so a hallucinated name is a schema
853+ // violation the model sees at once rather than a lookup failure it has to
854+ // reason about.
855+ type SkillArgs struct {
856+ Name string `json:"name" jsonschema:"description=Which skill to load"`
857+ Args string `json:"args,omitempty" jsonschema:"description=Arguments for the skill; whatever followed the skill name"`
858+ }
859+
860+ // NewSkill builds the skill tool over the skills discovered at startup. It
861+ // returns nil when there are none, so the caller registers nothing and the
862+ // prompt is byte-identical to a harness without the feature.
863+ func NewSkill(found []skills.Skill) Tool {
864+ if len(found) == 0 {
865+ return nil
866+ }
867+ byName := make(map[string]skills.Skill, len(found))
868+ names := make([]string, 0, len(found))
869+ var listing strings.Builder
870+ for _, s := range found {
871+ byName[s.Name] = s
872+ names = append(names, s.Name)
873+ fmt.Fprintf(&listing, "\n %s: %s", s.Name, s.Description)
874+ }
875+ sort.Strings(names)
876+
877+ desc := strings.TrimSpace(`
878+ Load a skill: a set of instructions for one kind of work. Invoke a skill when
879+ its description matches what you are about to do — before starting, not after.
880+ The instructions come back as the tool result; follow them.
881+
882+ Available skills:`) + listing.String()
883+
884+ t := New("skill", desc, func(_ context.Context, in SkillArgs) (string, error) {
885+ s, ok := byName[in.Name]
886+ if !ok {
887+ return "", fmt.Errorf("no skill named %q; available: %s", in.Name, strings.Join(names, ", "))
888+ }
889+ var b strings.Builder
890+ fmt.Fprintf(&b, "Skill: %s\nDirectory: %s\n", s.Name, s.Dir)
891+ b.WriteString("Relative paths below resolve against that directory.\n\n")
892+ b.WriteString(s.Body)
893+ if a := strings.TrimSpace(in.Args); a != "" {
894+ b.WriteString("\n\nARGUMENTS: " + a)
895+ }
896+ return b.String(), nil
897+ })
898+ return withEnum(t, "name", names)
899+ }
900+
901+ // withEnum patches an enum onto one string property of a reflected schema.
902+ // invopop/jsonschema derives the schema from struct tags, which cannot carry a
903+ // list only known at startup.
904+ func withEnum(t Tool, property string, values []string) Tool {
905+ var m map[string]any
906+ if err := json.Unmarshal(t.InputSchema(), &m); err != nil {
907+ return t
908+ }
909+ props, ok := m["properties"].(map[string]any)
910+ if !ok {
911+ return t
912+ }
913+ p, ok := props[property].(map[string]any)
914+ if !ok {
915+ return t
916+ }
917+ p["enum"] = values
918+ patched, err := json.Marshal(m)
919+ if err != nil {
920+ return t
921+ }
922+ return patchedSchema{Tool: t, schema: patched}
923+ }
924+
925+ // patchedSchema wraps a Tool to override only its input schema.
926+ type patchedSchema struct {
927+ Tool
928+ schema json.RawMessage
929+ }
930+
931+ func (p patchedSchema) InputSchema() json.RawMessage { return p.schema }
932+ ```
933+
934+ - [ ] **Step 4: Add `skill` to the auto-approved set**
935+
936+ In `internal/approval/policy.go`, the map at lines 40-42 becomes:
937+
938+ ```go
939+ "read_file": true,
940+ "glob": true,
941+ "grep": true,
942+ // skill reads one file from a directory found at startup, and cannot
943+ // reach anything read_file could not already reach.
944+ "skill": true,
945+ ```
946+
947+ - [ ] **Step 5: Run the tests to verify they pass**
948+
949+ Run: `go test ./internal/tools/ ./internal/approval/ -v`
950+ Expected: PASS.
951+
952+ - [ ] **Step 6: Commit**
953+
954+ ```bash
955+ cd /home/joni/Work/GO/coding-harness
956+ gofmt -w internal/tools internal/approval
957+ git add internal/tools internal/approval
958+ git commit -m "tools: a skill tool, auto-approved, with a slug enum"
959+ ```
960+
961+ ---
962+
963+ ### Task 4: Tell the model skills exist
964+
965+ **Files:**
966+ - Modify: `internal/agent/loop.go` (`Options` at line 42, `New` at line 112)
967+ - Test: `internal/agent/loop_test.go`
968+
969+ **Interfaces:**
970+ - Consumes: `skills.Skill` from Task 1.
971+ - Produces: `agent.Options.Skills []skills.Skill`. Task 6 sets it. The listing is appended to `opts.System` inside `New`, so `request()` is untouched and the system prompt is trivially constant for the session.
972+
973+ - [ ] **Step 1: Write the failing tests**
974+
975+ Add to `internal/agent/loop_test.go`:
976+
977+ ```go
978+ func TestSystemPromptListsSkills(t *testing.T) {
979+ a := New(nil, tools.NewRegistry(nil), NewState(), nil, Options{
980+ Skills: []skills.Skill{{Name: "commit-helper", Description: "Use when writing commit messages"}},
981+ })
982+ sys := a.opts.System
983+ if !strings.Contains(sys, "commit-helper: Use when writing commit messages") {
984+ t.Errorf("system prompt does not list the skill:\n%s", sys)
985+ }
986+ if !strings.Contains(sys, "/<name>") {
987+ t.Error("system prompt does not explain what a leading slash means")
988+ }
989+ if !strings.HasPrefix(sys, defaultSystem) {
990+ t.Error("the listing replaced the default prompt instead of being appended to it")
991+ }
992+ }
993+
994+ func TestSystemPromptUnchangedWithoutSkills(t *testing.T) {
995+ a := New(nil, tools.NewRegistry(nil), NewState(), nil, Options{})
996+ if a.opts.System != defaultSystem {
997+ t.Errorf("system prompt differs from the default with no skills:\n%s", a.opts.System)
998+ }
999+ }
1000+
1001+ func TestSystemPromptIsStableAcrossRequests(t *testing.T) {
1002+ // The listing sits in the cached prefix. If it varied per request, every
1003+ // turn would miss the cache.
1004+ a := New(nil, tools.NewRegistry(nil), NewState(), nil, Options{
1005+ Skills: []skills.Skill{{Name: "a", Description: "one"}, {Name: "b", Description: "two"}},
1006+ })
1007+ a.state.Append(llm.Message{Role: llm.RoleUser, Content: []llm.ContentBlock{llm.TextBlock("hi")}})
1008+ first := a.request().System[0].Text
1009+ second := a.request().System[0].Text
1010+ if first != second {
1011+ t.Error("the system prompt changed between two requests in one session")
1012+ }
1013+ }
1014+ ```
1015+
1016+ Add `"harness/internal/skills"` and `"strings"` to the test file's imports if they are not already there.
1017+
1018+ - [ ] **Step 2: Run the tests to verify they fail**
1019+
1020+ Run: `go test ./internal/agent/ -run SystemPrompt -v`
1021+ Expected: FAIL to compile — `unknown field Skills in struct literal`.
1022+
1023+ - [ ] **Step 3: Add the option and compose the prompt**
1024+
1025+ In `internal/agent/loop.go`, add to `Options`:
1026+
1027+ ```go
1028+ // Skills are the instruction sets discovered at startup. Their names and
1029+ // descriptions are appended to the system prompt; the bodies are loaded on
1030+ // demand by the skill tool.
1031+ Skills []skills.Skill
1032+ ```
1033+
1034+ Add `"harness/internal/skills"` and `"strings"` to the imports, and in `New`, right after the existing `if opts.System == "" { opts.System = defaultSystem }`:
1035+
1036+ ```go
1037+ opts.System += skillListing(opts.Skills)
1038+ ```
1039+
1040+ Then add, next to `defaultSystem`:
1041+
1042+ ```go
1043+ // skillListing is appended to the system prompt when skills were discovered.
1044+ // It is constant for the session, so the cached prefix still matches request to
1045+ // request: what breaks caching is content that varies per request, not content
1046+ // that varies per session.
1047+ func skillListing(found []skills.Skill) string {
1048+ if len(found) == 0 {
1049+ return ""
1050+ }
1051+ var b strings.Builder
1052+ b.WriteString("\n\nSkills are instruction sets for specific kinds of work. Invoke one with\nthe skill tool when its description matches what you are doing — before\nstarting, not after. A user message beginning with /<name> is a request to\ninvoke that skill; everything after the name is its arguments.\n\nAvailable skills:")
1053+ for _, s := range found {
1054+ fmt.Fprintf(&b, "\n %s: %s", s.Name, s.Description)
1055+ }
1056+ return b.String()
1057+ }
1058+ ```
1059+
1060+ `fmt` is already imported by `loop.go`; confirm before adding it again.
1061+
1062+ - [ ] **Step 4: Run the tests to verify they pass**
1063+
1064+ Run: `go test ./internal/agent/ -v`
1065+ Expected: PASS, including the existing compaction and loop tests.
1066+
1067+ - [ ] **Step 5: Commit**
1068+
1069+ ```bash
1070+ cd /home/joni/Work/GO/coding-harness
1071+ gofmt -w internal/agent
1072+ git add internal/agent
1073+ git commit -m "agent: list discovered skills in the cached system prompt"
1074+ ```
1075+
1076+ ---
1077+
1078+ ### Task 5: `/skills` and `/slug` in the TUI
1079+
1080+ **Files:**
1081+ - Modify: `internal/tui/model.go` (`submit` at line 426, `helpText` at line 546, `Options` struct)
1082+ - Test: `internal/tui/model_test.go`
1083+
1084+ **Interfaces:**
1085+ - Consumes: `skills.Skill` from Task 1.
1086+ - Produces: a `Skills []skills.Skill` field on the TUI's options struct. Task 6 sets it.
1087+
1088+ Read `internal/tui/model.go` around line 100 to find the options struct's real name and how `m.opts` is populated before writing this; the plan calls it `Options` but the field must go wherever `m.opts` comes from.
1089+
1090+ - [ ] **Step 1: Write the failing tests**
1091+
1092+ Add to `internal/tui/model_test.go`, following the shape of the existing `/help` test at line 231:
1093+
1094+ ```go
1095+ func TestSlashSkillsListsThem(t *testing.T) {
1096+ m := newTestModel(t)
1097+ m.opts.Skills = []skills.Skill{
1098+ {Name: "commit-helper", Description: "Use when writing commit messages", Source: "personal"},
1099+ }
1100+ m.submit("/skills")
1101+ out := m.tr.String()
1102+ if !strings.Contains(out, "commit-helper") || !strings.Contains(out, "Use when writing commit messages") {
1103+ t.Errorf("/skills did not list the skill:\n%s", out)
1104+ }
1105+ }
1106+
1107+ func TestSlashSkillsWithNoneSaysSo(t *testing.T) {
1108+ m := newTestModel(t)
1109+ m.submit("/skills")
1110+ if !strings.Contains(m.tr.String(), "no skills") {
1111+ t.Errorf("/skills with none found said nothing useful:\n%s", m.tr.String())
1112+ }
1113+ }
1114+
1115+ func TestKnownSkillIsSentAsAUserTurn(t *testing.T) {
1116+ m := newTestModel(t)
1117+ m.opts.Skills = []skills.Skill{{Name: "commit-helper", Description: "d"}}
1118+ m.submit("/commit-helper fix the flaky test")
1119+ if m.phase != phaseRunning {
1120+ t.Errorf("phase = %v, want phaseRunning — the line was not sent", m.phase)
1121+ }
1122+ if !strings.Contains(m.tr.String(), "/commit-helper fix the flaky test") {
1123+ t.Errorf("the user turn is not in the transcript:\n%s", m.tr.String())
1124+ }
1125+ }
1126+
1127+ func TestUnknownSlashCommandStillErrors(t *testing.T) {
1128+ m := newTestModel(t)
1129+ m.opts.Skills = []skills.Skill{{Name: "commit-helper", Description: "d"}}
1130+ m.submit("/commit-helpr")
1131+ if m.phase == phaseRunning {
1132+ t.Error("a typo was sent to the model instead of being caught")
1133+ }
1134+ if !strings.Contains(m.tr.String(), "unknown command") {
1135+ t.Errorf("no error for the typo:\n%s", m.tr.String())
1136+ }
1137+ }
1138+
1139+ func TestHelpMentionsSkills(t *testing.T) {
1140+ m := newTestModel(t)
1141+ m.submit("/help")
1142+ if !strings.Contains(m.tr.String(), "/skills") {
1143+ t.Error("/help does not mention /skills")
1144+ }
1145+ }
1146+ ```
1147+
1148+ `newTestModel` and the transcript-to-string helper may have different names — read the top of `internal/tui/model_test.go` and the existing `/help` test, and use whatever those tests already use rather than inventing new helpers.
1149+
1150+ - [ ] **Step 2: Run the tests to verify they fail**
1151+
1152+ Run: `go test ./internal/tui/ -run 'Skill|Help' -v`
1153+ Expected: FAIL — `m.opts.Skills` undefined.
1154+
1155+ - [ ] **Step 3: Add the field and the two cases**
1156+
1157+ Add to the TUI options struct:
1158+
1159+ ```go
1160+ // Skills are the instruction sets discovered at startup, used to resolve
1161+ // /<slug> and to answer /skills.
1162+ Skills []skills.Skill
1163+ ```
1164+
1165+ In `submit`, add the `/skills` case anywhere among the existing command cases:
1166+
1167+ ```go
1168+ case line == "/skills":
1169+ if len(m.opts.Skills) == 0 {
1170+ m.tr.addNotice("no skills found in ~/" + skills.Dirname + " or ./" + skills.Dirname)
1171+ } else {
1172+ var b strings.Builder
1173+ b.WriteString("skills:")
1174+ for _, s := range m.opts.Skills {
1175+ fmt.Fprintf(&b, "\n /%s (%s) \u2014 %s", s.Name, s.Source, s.Description)
1176+ }
1177+ m.tr.addNotice(b.String())
1178+ }
1179+ return m, m.sync()
1180+ ```
1181+
1182+ Then make one change to the existing unknown-command case at line 448 — add the
1183+ guard, and nothing else:
1184+
1185+ ```go
1186+ case strings.HasPrefix(line, "/") && !m.isSkill(line):
1187+ m.tr.addError("unknown command " + line + " — /help lists them")
1188+ return m, m.sync()
1189+ }
1190+ ```
1191+
1192+ A recognised skill now matches no case, falls out of the switch, and reaches
1193+ the existing user-turn code at the bottom of `submit` untouched. The model sees
1194+ the slash, the system prompt explains what it means, and the skill tool does
1195+ the loading — one code path for both ways in.
1196+
1197+ Add the helper next to `submit`:
1198+
1199+ ```go
1200+ // isSkill reports whether a slash command names a discovered skill. Matching
1201+ // here rather than sending every unrecognised slash to the model means a typo
1202+ // is caught by the harness instead of puzzled over by the agent.
1203+ func (m *model) isSkill(line string) bool {
1204+ name := strings.TrimPrefix(line, "/")
1205+ if i := strings.IndexAny(name, " \t"); i >= 0 {
1206+ name = name[:i]
1207+ }
1208+ for _, s := range m.opts.Skills {
1209+ if s.Name == name {
1210+ return true
1211+ }
1212+ }
1213+ return false
1214+ }
1215+ ```
1216+
1217+ Update `helpText` at line 546:
1218+
1219+ ```go
1220+ const helpText = `commands: /allowed /clear /help /skills /exit
1221+ ```
1222+
1223+ Keep the rest of that const exactly as it is. Add `"harness/internal/skills"` and `"fmt"` to the imports if absent.
1224+
1225+ - [ ] **Step 4: Run the tests to verify they pass**
1226+
1227+ Run: `go test ./internal/tui/ -v`
1228+ Expected: PASS, including the existing look and colour tests.
1229+
1230+ - [ ] **Step 5: Commit**
1231+
1232+ ```bash
1233+ cd /home/joni/Work/GO/coding-harness
1234+ gofmt -w internal/tui
1235+ git add internal/tui
1236+ git commit -m "tui: /skills, and /<slug> as a user turn"
1237+ ```
1238+
1239+ ---
1240+
1241+ ### Task 6: Wire it up, and document it
1242+
1243+ **Files:**
1244+ - Modify: `cmd/harness/main.go` (flags near line 55, wiring at lines 101-130)
1245+ - Modify: `README.md`
1246+ - Test: manual, per the steps below
1247+
1248+ **Interfaces:**
1249+ - Consumes: everything from Tasks 1-5.
1250+ - Produces: the working feature. Nothing depends on this task.
1251+
1252+ - [ ] **Step 1: Add the flag**
1253+
1254+ In `cmd/harness/main.go`, after the `-no-tui` flag at line 55:
1255+
1256+ ```go
1257+ noSkills := flag.Bool("no-skills", false, "skip discovering skills in ~/"+skills.Dirname+" and ./"+skills.Dirname)
1258+ ```
1259+
1260+ - [ ] **Step 2: Discover, and wire the results through**
1261+
1262+ Replace the registry construction at lines 101-109:
1263+
1264+ ```go
1265+ var found []skills.Skill
1266+ if !*noSkills {
1267+ home, err := os.UserHomeDir()
1268+ if err != nil {
1269+ // No home directory is not fatal: project skills still work.
1270+ home = ""
1271+ }
1272+ var problems []error
1273+ found, problems = skills.Discover(home, root)
1274+ for _, p := range problems {
1275+ notice("skill skipped: %v", p)
1276+ }
1277+ }
1278+
1279+ // Skill directories are readable by the search tools and by nothing else,
1280+ // so a skill can read the files it ships with.
1281+ var skillDirs []string
1282+ for _, s := range found {
1283+ skillDirs = append(skillDirs, s.Dir)
1284+ }
1285+
1286+ state := agent.NewState()
1287+ reg := tools.NewRegistry(log).Register(
1288+ tools.NewRead(root, state, skillDirs...),
1289+ tools.NewGlob(root, skillDirs...),
1290+ tools.NewGrep(root, skillDirs...),
1291+ tools.NewEdit(root, state),
1292+ tools.NewWrite(root, state),
1293+ tools.NewBash(root),
1294+ )
1295+ if t := tools.NewSkill(found); t != nil {
1296+ reg.Register(t)
1297+ }
1298+ ```
1299+
1300+ Add `Skills: found,` to the `agent.Options` literal at line 123.
1301+
1302+ Find where the TUI options are built further down `main.go` (search for the struct the TUI's `m.opts` comes from) and add `Skills: found,` there too.
1303+
1304+ Confirm `"harness/internal/skills"` and `"os"` are imported.
1305+
1306+ - [ ] **Step 3: Build and run the whole suite**
1307+
1308+ Run: `go build ./... && go test ./...`
1309+ Expected: PASS.
1310+
1311+ - [ ] **Step 4: Verify the feature by hand against the mock API**
1312+
1313+ ```bash
1314+ cd /home/joni/Work/GO/coding-harness
1315+ mkdir -p /tmp/skilldemo/.agents/skills/demo-skill/references
1316+ cat > /tmp/skilldemo/.agents/skills/demo-skill/SKILL.md <<'EOF'
1317+ ---
1318+ name: demo-skill
1319+ description: Use when demonstrating that skills load
1320+ ---
1321+ Say the word BANANA, then read references/note.md and say what it contains.
1322+ EOF
1323+ echo "the note says pineapple" > /tmp/skilldemo/.agents/skills/demo-skill/references/note.md
1324+ git -C /tmp/skilldemo init -q
1325+ go build -o harness ./cmd/harness
1326+ ./harness -C /tmp/skilldemo -no-tui -no-skills -p "/demo-skill" 2>&1 | head -5
1327+ ```
1328+
1329+ Expected with `-no-skills`: the model is told nothing about skills and has no tool, so it treats `/demo-skill` as ordinary text.
1330+
1331+ Then, with a real key and without the flag:
1332+
1333+ ```bash
1334+ ./harness -C /tmp/skilldemo -no-tui -p "/demo-skill"
1335+ ```
1336+
1337+ Expected: a `Skill(name=demo-skill)` call, then a `ReadFile` of the bundled `references/note.md`, then a reply mentioning both BANANA and pineapple. The read of `references/note.md` is the proof that the jail change works.
1338+
1339+ - [ ] **Step 5: Verify `/skills` in the TUI**
1340+
1341+ ```bash
1342+ ./harness -C /tmp/skilldemo
1343+ ```
1344+
1345+ Type `/skills` — it must list `demo-skill (project)`. Type `/demo-skil` — it must say `unknown command`. Type `/help` — it must list `/skills`.
1346+
1347+ - [ ] **Step 6: Document it in the README**
1348+
1349+ Add `/skills` to the commands line (`Commands: /allowed, /clear, /help, /exit`) and `-no-skills` to the useful-flags line. Add a `## Skills` section after `## Tools`:
1350+
1351+ ````markdown
1352+ ## Skills
1353+
1354+ A skill is a directory of instructions the agent pulls in when it is relevant:
1355+
1356+ ```
1357+ ~/.agents/skills/commit-helper/SKILL.md every repo
1358+ ./.agents/skills/commit-helper/SKILL.md this one, checked in
1359+ ```
1360+
1361+ ```markdown
1362+ ---
1363+ name: commit-helper
1364+ description: Use when writing commit messages for this repo
1365+ ---
1366+
1367+ Read the last twenty commits for tone before writing anything.
1368+ Conventions are in references/conventions.md.
1369+ ```
1370+
1371+ The directory name is the skill's name, and a `name:` that disagrees with it is
1372+ a load error. Only the description reaches the model until the skill is
1373+ invoked, so it is what decides whether the skill fires at all; the body can be
1374+ as long as you like.
1375+
1376+ Two ways in, one way to load. The agent calls the `skill` tool when a
1377+ description matches what it is doing; you can force one with `/commit-helper
1378+ fix the flaky test`, which the model answers by making the same call. `/skills`
1379+ lists what was found, project shadowing personal on a name collision.
1380+
1381+ Skill directories are readable by `read_file`, `glob` and `grep` even though
1382+ they sit outside the working root, so a skill can ship reference files and
1383+ point at them. They are not writable by anything: `edit_file`, `write_file` and
1384+ `bash` keep the working root as their only root. A skill is input, not a work
1385+ product.
1386+
1387+ Discovery happens at startup. The name-and-description listing goes in the
1388+ cached prompt prefix, so re-reading the directory each turn would cost a cache
1389+ miss on every turn for a file that never changes. Restart to pick up a new
1390+ skill. `-no-skills` turns the whole thing off; the eval runner never discovers
1391+ skills at all, so METRICS.md is unaffected by what is in your home directory.
1392+ ````
1393+
1394+ Add `internal/skills discovery and parsing of SKILL.md` to the layout block near the end of the README.
1395+
1396+ - [ ] **Step 7: Commit**
1397+
1398+ ```bash
1399+ cd /home/joni/Work/GO/coding-harness
1400+ gofmt -w ./cmd ./internal
1401+ go build ./... && go test ./...
1402+ git add cmd README.md
1403+ git commit -m "harness: discover and wire up skills, document them"
1404+ ```
1405+
1406+ ---
1407+
1408+ ## Self-Review
1409+
1410+ **Spec coverage:** Format → Task 1. Discovery and precedence → Task 1. The `skill` tool, enum, preamble, `ARGUMENTS:`, repeat invocation → Task 3. System prompt listing and cache stability → Task 4. User invocation, `/skills`, `/help`, line-based front end → Task 5. Path jail and the per-tool table → Task 2. Flags → Task 6. Dependency → Task 1 Step 1. Package shape → Task 1. Testing → the test steps in Tasks 1-5, plus manual verification in Task 6. Out of scope → nothing planned. No gaps.
1411+
1412+ **Type consistency:** `skills.Skill{Name, Description, Body, Dir, Source}` and `skills.Discover(home, workroot) ([]Skill, []error)` are defined in Task 1 and used with those exact names in Tasks 3, 4, 5 and 6. `tools.NewSkill(found []skills.Skill) Tool` is defined in Task 3 and called in Task 6. `jailWith(root, extra, path)` is defined in Task 2 and used in Tasks 2 only. `skills.Dirname` is defined in Task 1 and used in Tasks 5 and 6. The variadic constructors from Task 2 are called with `skillDirs...` in Task 6.
1413+
1414+ **Known soft spots for the implementer:** Tasks 3 and 5 depend on names this plan could not read — the `approval.Request`/`Decide` shape in `policy_test.go`, the TUI options struct's real name, and the `newTestModel`/transcript helpers in `model_test.go`. Each step says to read the neighbouring code and match it rather than trusting the plan's guess.
+11−7docs/superpowers/specs/2026-09-17-agents-skills-design.mdView File
@@ -176,10 +176,14 @@ arbitrary files under `~` that happen to sit next to the skills directory.
176176 ## Flags
177177
178178 `-no-skills` skips discovery entirely: no tool, no system prompt addition, no
179- extra jail roots. The eval runner sets it, so a task's token counts do not move
180- because of what happens to be in the developer's home directory. METRICS.md
181- compares runs, and that only means something if the prompt prefix is the same
182- between them.
179+ extra jail roots. It exists so a developer can measure what skills cost, and as
180+ the off switch when a skill is misbehaving.
181+
182+ The eval runner needs no flag. It builds its own registry at `evals/runner.go:191`
183+ and simply never calls `Discover`, so a task's token counts cannot move because
184+ of what happens to be in the developer's home directory. METRICS.md compares
185+ runs, and that only means something if the prompt prefix is the same between
186+ them.
183187
184188 ## Dependency
185189
@@ -203,9 +207,9 @@ type Skill struct {
203207 Source string // "personal" | "project"
204208 }
205209
206- // Discover scans the personal and project roots. Malformed skills are
207- // returned as problems rather than errors; only an unreadable root is an error.
208- func Discover(home, workroot string) (found []Skill, problems []error, err error)
210+ // Discover scans the personal and project roots. Every failure is a problem,
211+ // never an error: nothing about skills prevents the harness from starting.
212+ func Discover(home, workroot string) (found []Skill, problems []error)
209213 ```
210214
211215 `home` and `workroot` are parameters rather than looked up inside, so tests