# Skills Implementation Plan

> **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.

**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`.

**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.

**Tech Stack:** Go 1.26, `gopkg.in/yaml.v3` (new), `invopop/jsonschema` (existing, for the tool schema), Bubble Tea (existing, for the TUI).

**Spec:** `docs/superpowers/specs/2026-09-17-agents-skills-design.md`

## Global Constraints

- Go module is `harness`; import paths are `harness/internal/...`.
- `internal/agent` must never import `internal/ui` or `internal/tui`. It may import `internal/skills`.
- Every tool returns `(string, error)` where the error is text the model reads.
- Slugs match `^[a-z0-9][a-z0-9-]*$`.
- Frontmatter `name` must equal the directory name, or the skill is a problem.
- Skills are readable, never writable: `edit_file`, `write_file` and `bash` get no extra roots.
- Nothing about skills aborts startup. Every discovery failure is a problem, not an error.
- Discovery is startup-only. No hot reload.
- 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.
- New exported identifiers get a doc comment, matching the existing house style.
- Run `gofmt -w` on every file you touch before committing.

---

### Task 1: The `internal/skills` package

**Files:**
- Create: `internal/skills/skills.go`
- Create: `internal/skills/skills_test.go`
- Modify: `go.mod`, `go.sum`

**Interfaces:**
- Consumes: nothing.
- 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`.

- [ ] **Step 1: Add the YAML dependency**

```bash
cd /home/joni/Work/GO/coding-harness
go get gopkg.in/yaml.v3@v3.0.1
```

- [ ] **Step 2: Write the failing tests**

Create `internal/skills/skills_test.go`:

```go
package skills

import (
	"os"
	"path/filepath"
	"strings"
	"testing"
)

// write creates <root>/<slug>/SKILL.md with the given contents.
func write(t *testing.T, root, slug, body string) string {
	t.Helper()
	dir := filepath.Join(root, slug)
	if err := os.MkdirAll(dir, 0o755); err != nil {
		t.Fatal(err)
	}
	if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(body), 0o644); err != nil {
		t.Fatal(err)
	}
	return dir
}

const wellFormed = `---
name: commit-helper
description: Use when writing commit messages
---

Read the last twenty commits for tone.
`

func TestDiscoverWellFormed(t *testing.T) {
	home, work := t.TempDir(), t.TempDir()
	dir := write(t, filepath.Join(home, ".agents", "skills"), "commit-helper", wellFormed)

	found, problems := Discover(home, work)
	if len(problems) != 0 {
		t.Fatalf("problems: %v", problems)
	}
	if len(found) != 1 {
		t.Fatalf("got %d skills, want 1", len(found))
	}
	s := found[0]
	if s.Name != "commit-helper" {
		t.Errorf("Name = %q", s.Name)
	}
	if s.Description != "Use when writing commit messages" {
		t.Errorf("Description = %q", s.Description)
	}
	if s.Source != "personal" {
		t.Errorf("Source = %q, want personal", s.Source)
	}
	if s.Dir != dir {
		t.Errorf("Dir = %q, want %q", s.Dir, dir)
	}
	if !strings.Contains(s.Body, "last twenty commits") {
		t.Errorf("Body = %q", s.Body)
	}
	if strings.Contains(s.Body, "description:") {
		t.Error("Body still contains the frontmatter")
	}
}

func TestProjectShadowsPersonal(t *testing.T) {
	home, work := t.TempDir(), t.TempDir()
	write(t, filepath.Join(home, ".agents", "skills"), "dup", `---
name: dup
description: personal one
---
personal body
`)
	write(t, filepath.Join(work, ".agents", "skills"), "dup", `---
name: dup
description: project one
---
project body
`)

	found, problems := Discover(home, work)
	if len(problems) != 0 {
		t.Fatalf("problems: %v", problems)
	}
	if len(found) != 1 {
		t.Fatalf("got %d skills, want 1", len(found))
	}
	if found[0].Description != "project one" || found[0].Source != "project" {
		t.Errorf("personal skill was not shadowed: %+v", found[0])
	}
}

func TestMalformedSkillsAreProblemsAndDoNotBlockOthers(t *testing.T) {
	home, work := t.TempDir(), t.TempDir()
	root := filepath.Join(home, ".agents", "skills")
	write(t, root, "good", wellFormed2())
	write(t, root, "no-frontmatter", "just a body, no frontmatter\n")
	write(t, root, "no-description", "---\nname: no-description\n---\nbody\n")
	write(t, root, "wrong-name", "---\nname: something-else\ndescription: d\n---\nbody\n")
	write(t, root, "Bad_Slug", "---\nname: Bad_Slug\ndescription: d\n---\nbody\n")

	found, problems := Discover(home, work)
	if len(found) != 1 || found[0].Name != "good" {
		t.Fatalf("got %+v, want only the good skill", found)
	}
	if len(problems) != 4 {
		t.Fatalf("got %d problems, want 4: %v", len(problems), problems)
	}
	// Every problem names the offending path, or it is not actionable.
	for _, p := range problems {
		if !strings.Contains(p.Error(), root) {
			t.Errorf("problem does not name a path: %v", p)
		}
	}
}

func wellFormed2() string {
	return "---\nname: good\ndescription: a good skill\n---\nbody\n"
}

func TestDescriptionScalarStyles(t *testing.T) {
	// A folded and a quoted description must parse to the same text as a plain
	// one. This is the whole reason for the yaml dependency.
	cases := map[string]string{
		"plain":  "description: Use when writing commit messages",
		"quoted": `description: "Use when writing commit messages"`,
		"folded": "description: >-\n  Use when writing\n  commit messages",
	}
	for slug, line := range cases {
		t.Run(slug, func(t *testing.T) {
			home, work := t.TempDir(), t.TempDir()
			write(t, filepath.Join(home, ".agents", "skills"), slug,
				"---\nname: "+slug+"\n"+line+"\n---\nbody\n")

			found, problems := Discover(home, work)
			if len(problems) != 0 {
				t.Fatalf("problems: %v", problems)
			}
			if len(found) != 1 || found[0].Description != "Use when writing commit messages" {
				t.Fatalf("Description = %+v", found)
			}
		})
	}
}

func TestNoRootsIsSilent(t *testing.T) {
	found, problems := Discover(t.TempDir(), t.TempDir())
	if len(found) != 0 || len(problems) != 0 {
		t.Fatalf("found %v, problems %v; want both empty", found, problems)
	}
}

func TestNestedSkillIsNotDiscovered(t *testing.T) {
	home, work := t.TempDir(), t.TempDir()
	write(t, filepath.Join(home, ".agents", "skills", "outer"), "inner", wellFormed2())

	found, problems := Discover(home, work)
	if len(found) != 0 {
		t.Fatalf("found %+v, want none", found)
	}
	// "outer" has no SKILL.md of its own; that is not a problem, just not a skill.
	if len(problems) != 0 {
		t.Fatalf("problems: %v", problems)
	}
}

func TestSortedByName(t *testing.T) {
	home, work := t.TempDir(), t.TempDir()
	root := filepath.Join(home, ".agents", "skills")
	for _, slug := range []string{"zebra", "alpha", "middle"} {
		write(t, root, slug, "---\nname: "+slug+"\ndescription: d\n---\nbody\n")
	}
	found, _ := Discover(home, work)
	var got []string
	for _, s := range found {
		got = append(got, s.Name)
	}
	if strings.Join(got, ",") != "alpha,middle,zebra" {
		t.Errorf("order = %v", got)
	}
}
```

- [ ] **Step 3: Run the tests to verify they fail**

Run: `go test ./internal/skills/`
Expected: FAIL — `undefined: Discover`, `undefined: Skill`.

- [ ] **Step 4: Write the implementation**

Create `internal/skills/skills.go`:

```go
// Package skills discovers instruction sets stored as Markdown on disk.
//
// A skill is a directory containing SKILL.md: YAML frontmatter naming and
// describing it, followed by the instructions. The description is all the
// model sees until the skill is invoked, so it carries the whole triggering
// decision; the body can be any length.
package skills

import (
	"fmt"
	"os"
	"path/filepath"
	"regexp"
	"sort"
	"strings"

	"gopkg.in/yaml.v3"
)

// Dirname is the directory, under the home directory and under the working
// root, that skills are read from.
const Dirname = ".agents/skills"

// filename is the one file a skill directory must contain.
const filename = "SKILL.md"

// slugPattern is what a directory name must match to be a skill. The slug
// becomes a slash command and a JSON enum value, and neither wants spaces.
var slugPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`)

// Skill is one discovered instruction set.
type Skill struct {
	Name        string // slug, taken from the directory name
	Description string // the one line the model sees before invoking
	Body        string // SKILL.md with the frontmatter removed
	Dir         string // absolute path to the skill directory
	Source      string // "personal" or "project"
}

// frontmatter is the subset of SKILL.md's YAML header this harness reads.
// Unknown keys are ignored rather than rejected, so a skill written for
// another harness still loads here.
type frontmatter struct {
	Name        string `yaml:"name"`
	Description string `yaml:"description"`
}

// Discover scans the personal and project roots, project shadowing personal on
// a slug collision, and returns the skills sorted by name.
//
// Every failure is a problem rather than an error: a harness that will not open
// because of a stray file in the home directory is worse than one that runs
// without the skill it was going to load. A root that does not exist is not
// even a problem — neither exists on a fresh machine, and that is the normal
// case.
func Discover(home, workroot string) ([]Skill, []error) {
	byName := map[string]Skill{}
	var problems []error

	// Later roots win, so the project root is scanned second.
	for _, r := range []struct{ dir, source string }{
		{filepath.Join(home, Dirname), "personal"},
		{filepath.Join(workroot, Dirname), "project"},
	} {
		found, probs := scan(r.dir, r.source)
		problems = append(problems, probs...)
		for _, s := range found {
			byName[s.Name] = s
		}
	}

	out := make([]Skill, 0, len(byName))
	for _, s := range byName {
		out = append(out, s)
	}
	sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name })
	return out, problems
}

// scan reads one root, one level deep.
func scan(root, source string) ([]Skill, []error) {
	entries, err := os.ReadDir(root)
	if os.IsNotExist(err) {
		return nil, nil
	}
	if err != nil {
		return nil, []error{fmt.Errorf("%s: %w", root, err)}
	}

	var found []Skill
	var problems []error
	for _, e := range entries {
		if !e.IsDir() {
			continue
		}
		dir := filepath.Join(root, e.Name())
		path := filepath.Join(dir, filename)
		if _, err := os.Stat(path); err != nil {
			// A directory without a SKILL.md is not a skill, and not a
			// complaint either: it may be anything at all.
			continue
		}
		s, err := load(dir, e.Name(), source)
		if err != nil {
			problems = append(problems, err)
			continue
		}
		found = append(found, s)
	}
	return found, problems
}

// load parses one SKILL.md. slug is the directory name, which is the authority
// for the skill's name.
func load(dir, slug, source string) (Skill, error) {
	path := filepath.Join(dir, filename)
	if !slugPattern.MatchString(slug) {
		return Skill{}, fmt.Errorf("%s: directory name %q is not a valid skill name; use lower-case letters, digits and hyphens", path, slug)
	}
	raw, err := os.ReadFile(path)
	if err != nil {
		return Skill{}, fmt.Errorf("%s: %w", path, err)
	}
	head, body, err := split(string(raw))
	if err != nil {
		return Skill{}, fmt.Errorf("%s: %w", path, err)
	}
	var fm frontmatter
	if err := yaml.Unmarshal([]byte(head), &fm); err != nil {
		return Skill{}, fmt.Errorf("%s: frontmatter is not valid YAML: %w", path, err)
	}
	switch {
	case strings.TrimSpace(fm.Description) == "":
		return Skill{}, fmt.Errorf("%s: frontmatter has no description; it is the only thing the model sees before invoking the skill", path)
	case strings.TrimSpace(fm.Name) == "":
		return Skill{}, fmt.Errorf("%s: frontmatter has no name; it must be %q, matching the directory", path, slug)
	case strings.TrimSpace(fm.Name) != slug:
		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)
	}
	abs, err := filepath.Abs(dir)
	if err != nil {
		abs = dir
	}
	return Skill{
		Name:        slug,
		Description: strings.TrimSpace(fm.Description),
		Body:        strings.TrimSpace(body),
		Dir:         abs,
		Source:      source,
	}, nil
}

// split separates the leading --- fenced YAML block from the body.
func split(s string) (head, body string, err error) {
	s = strings.ReplaceAll(s, "\r\n", "\n")
	if !strings.HasPrefix(s, "---\n") {
		return "", "", fmt.Errorf("does not begin with a --- frontmatter block")
	}
	rest := s[len("---\n"):]
	end := strings.Index(rest, "\n---")
	if end < 0 {
		return "", "", fmt.Errorf("frontmatter block is never closed with ---")
	}
	head = rest[:end]
	body = rest[end+len("\n---"):]
	if i := strings.IndexByte(body, '\n'); i >= 0 {
		body = body[i+1:]
	} else {
		body = ""
	}
	return head, body, nil
}
```

- [ ] **Step 5: Run the tests to verify they pass**

Run: `go test ./internal/skills/ -v`
Expected: PASS, all cases.

- [ ] **Step 6: Commit**

```bash
cd /home/joni/Work/GO/coding-harness
gofmt -w internal/skills/
go mod tidy
git add internal/skills go.mod go.sum
git commit -m "skills: discover SKILL.md under ~/.agents/skills and .agents/skills"
```

---

### Task 2: Read-only extra roots in the path jail

**Files:**
- Modify: `internal/tools/read.go` (`jail` at line 141, `resolve` at line 114, `NewRead` at line 40)
- Modify: `internal/tools/glob.go:45`
- Modify: `internal/tools/grep.go:74`
- Test: `internal/tools/jail_test.go`

**Interfaces:**
- Consumes: nothing from Task 1.
- 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.

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.

- [ ] **Step 1: Write the failing tests**

Append to `internal/tools/jail_test.go`:

```go
// A skill lives outside the working root. The read tools reach into it; the
// write tools must not.
func TestExtraRootsAreReadableNotWritable(t *testing.T) {
	root := t.TempDir()
	skill := t.TempDir()
	ref := filepath.Join(skill, "references.md")
	if err := os.WriteFile(ref, []byte("conventions\n"), 0o644); err != nil {
		t.Fatal(err)
	}

	read := NewRead(root, nil, skill)
	out, err := read.Execute(context.Background(), jsonArgs(t, map[string]any{"path": ref}))
	if err != nil {
		t.Fatalf("read_file refused an extra root: %v", err)
	}
	if !strings.Contains(out, "conventions") {
		t.Errorf("read_file returned %q", out)
	}

	write := NewWrite(root, nil)
	if _, err := write.Execute(context.Background(), jsonArgs(t, map[string]any{
		"path": ref, "content": "rewritten",
	})); err == nil {
		t.Error("write_file was allowed into a skill directory")
	}
}

func TestExtraRootsDoNotWidenTheJail(t *testing.T) {
	root := t.TempDir()
	skill := t.TempDir()
	outside := filepath.Join(t.TempDir(), "secret.txt")
	if err := os.WriteFile(outside, []byte("secret\n"), 0o644); err != nil {
		t.Fatal(err)
	}

	read := NewRead(root, nil, skill)
	if _, err := read.Execute(context.Background(), jsonArgs(t, map[string]any{"path": outside})); err == nil {
		t.Error("a path under neither the root nor an extra root was allowed")
	}
}

func TestExtraRootSymlinkCannotStepSideways(t *testing.T) {
	root := t.TempDir()
	skill := t.TempDir()
	outside := t.TempDir()
	if err := os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret\n"), 0o644); err != nil {
		t.Fatal(err)
	}
	link := filepath.Join(skill, "escape")
	if err := os.Symlink(outside, link); err != nil {
		t.Skipf("symlinks unavailable: %v", err)
	}

	read := NewRead(root, nil, skill)
	if _, err := read.Execute(context.Background(), jsonArgs(t, map[string]any{
		"path": filepath.Join(link, "secret.txt"),
	})); err == nil {
		t.Error("a symlink out of a skill directory was followed")
	}
}

func TestGlobAndGrepAcceptExtraRoots(t *testing.T) {
	root := t.TempDir()
	skill := t.TempDir()
	if err := os.WriteFile(filepath.Join(skill, "notes.md"), []byte("needle\n"), 0o644); err != nil {
		t.Fatal(err)
	}

	g := NewGlob(root, skill)
	if _, err := g.Execute(context.Background(), jsonArgs(t, map[string]any{
		"pattern": "*.md", "path": skill,
	})); err != nil {
		t.Errorf("glob refused an extra root: %v", err)
	}

	gr := NewGrep(root, skill)
	out, err := gr.Execute(context.Background(), jsonArgs(t, map[string]any{
		"pattern": "needle", "path": skill,
	}))
	if err != nil {
		t.Errorf("grep refused an extra root: %v", err)
	}
	if !strings.Contains(out, "needle") {
		t.Errorf("grep returned %q", out)
	}
}
```

If `jsonArgs` does not already exist in the `tools` test files, add it to `jail_test.go`:

```go
// jsonArgs marshals a tool's arguments, failing the test rather than the tool.
func jsonArgs(t *testing.T, m map[string]any) json.RawMessage {
	t.Helper()
	b, err := json.Marshal(m)
	if err != nil {
		t.Fatal(err)
	}
	return b
}
```

Check first — run `grep -rn "func jsonArgs" internal/tools/` and reuse whatever is there, matching its existing signature.

- [ ] **Step 2: Run the tests to verify they fail**

Run: `go test ./internal/tools/ -run 'ExtraRoot|GlobAndGrep' -v`
Expected: FAIL to compile — `too many arguments in call to NewRead`.

- [ ] **Step 3: Replace `jail` with `jailWith` in `internal/tools/read.go`**

Replace the existing `jail` function (line 141) with the two below, keeping the original doc comment's intent:

```go
// jail refuses paths outside the working root. Symlinks are resolved first, on
// the existing part of the path, so a link pointing out of the tree cannot be
// used to step around the check. It is cheap and catches a lot; it is not a
// substitute for a sandbox, since bash can still cd anywhere.
func jail(root, path string) (string, error) {
	return jailWith(root, nil, path)
}

// jailWith is jail with additional permitted roots, used by the read-only
// tools so a skill can read the files it ships with. Each extra root gets the
// same symlink resolution as the working root, so an extra root cannot be used
// to step sideways into the rest of the filesystem.
func jailWith(root string, extra []string, path string) (string, error) {
	if root == "" {
		return path, nil
	}
	// A file being created does not exist yet, so resolve the deepest existing
	// ancestor and re-attach the rest.
	probe, rest := path, ""
	for {
		if resolved, err := filepath.EvalSymlinks(probe); err == nil {
			path = filepath.Join(resolved, rest)
			break
		}
		parent := filepath.Dir(probe)
		if parent == probe {
			path = filepath.Clean(path)
			break
		}
		rest = filepath.Join(filepath.Base(probe), rest)
		probe = parent
	}

	realRoot := realpath(root)
	if contains(realRoot, path) {
		return path, nil
	}
	for _, e := range extra {
		if e != "" && contains(realpath(e), path) {
			return path, nil
		}
	}
	if len(extra) > 0 {
		return "", fmt.Errorf("%s is outside the working root %s and every skill directory; this harness only touches files under the root", path, realRoot)
	}
	return "", fmt.Errorf("%s is outside the working root %s; this harness only touches files under the root", path, realRoot)
}

// realpath resolves symlinks, falling back to a lexical clean for a path that
// does not exist.
func realpath(p string) string {
	if resolved, err := filepath.EvalSymlinks(p); err == nil {
		return resolved
	}
	return filepath.Clean(p)
}

// contains reports whether path is root or sits beneath it.
func contains(root, path string) bool {
	rel, err := filepath.Rel(root, path)
	if err != nil {
		return false
	}
	return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
}
```

- [ ] **Step 4: Thread extra roots through the three read tools**

In `internal/tools/read.go`, change `resolve` and `NewRead`:

```go
// resolve anchors a relative path to root and refuses anything outside it and
// outside every extra root.
func resolve(root string, extra []string, path string) (string, error) {
	if path == "" {
		return "", fmt.Errorf("path is required")
	}
	return jailWith(root, extra, resolveBase(root, path))
}

// NewRead builds the read_file tool. root anchors relative paths; track may be
// nil; extra names additional directories that may be read but never written.
func NewRead(root string, track ReadTracker, extra ...string) Tool {
```

and inside its closure, line 49 becomes:

```go
			path, err := resolve(root, extra, in.Path)
```

`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.

In `internal/tools/glob.go`, change the signature to `func NewGlob(root string, extra ...string) Tool` and line 45 to:

```go
			base, err := jailWith(root, extra, resolveBase(root, in.Path))
```

In `internal/tools/grep.go`, change the signature to `func NewGrep(root string, extra ...string) Tool` and line 74 to:

```go
			base, err := jailWith(root, extra, resolveBase(root, in.Path))
```

- [ ] **Step 5: Run the whole tools suite**

Run: `go test ./internal/tools/`
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.

- [ ] **Step 6: Verify nothing else broke**

Run: `go build ./... && go test ./...`
Expected: PASS. Existing call sites compile because the new parameters are variadic.

- [ ] **Step 7: Commit**

```bash
cd /home/joni/Work/GO/coding-harness
gofmt -w internal/tools/
git add internal/tools
git commit -m "tools: read-only extra roots in the path jail"
```

---

### Task 3: The `skill` tool

**Files:**
- Create: `internal/tools/skill.go`
- Create: `internal/tools/skill_test.go`
- Modify: `internal/approval/policy.go:40-42`
- Test: `internal/approval/policy_test.go`

**Interfaces:**
- Consumes: `skills.Skill` and `skills.Discover` from Task 1.
- 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.

- [ ] **Step 1: Write the failing tests**

Create `internal/tools/skill_test.go`:

```go
package tools

import (
	"context"
	"encoding/json"
	"strings"
	"testing"

	"harness/internal/skills"
)

func testSkills() []skills.Skill {
	return []skills.Skill{
		{Name: "alpha", Description: "the first one", Body: "do the alpha thing", Dir: "/skills/alpha", Source: "personal"},
		{Name: "beta", Description: "the second one", Body: "do the beta thing", Dir: "/skills/beta", Source: "project"},
	}
}

func TestSkillReturnsBodyWithDirectory(t *testing.T) {
	tool := NewSkill(testSkills())
	out, err := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "alpha"}))
	if err != nil {
		t.Fatal(err)
	}
	for _, want := range []string{"Skill: alpha", "Directory: /skills/alpha", "do the alpha thing"} {
		if !strings.Contains(out, want) {
			t.Errorf("output missing %q:\n%s", want, out)
		}
	}
}

func TestSkillAppendsArguments(t *testing.T) {
	tool := NewSkill(testSkills())
	out, err := tool.Execute(context.Background(), jsonArgs(t, map[string]any{
		"name": "beta", "args": "fix the flaky test",
	}))
	if err != nil {
		t.Fatal(err)
	}
	if !strings.Contains(out, "ARGUMENTS: fix the flaky test") {
		t.Errorf("arguments missing:\n%s", out)
	}
}

func TestSkillOmitsArgumentsLineWhenEmpty(t *testing.T) {
	tool := NewSkill(testSkills())
	out, _ := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "alpha"}))
	if strings.Contains(out, "ARGUMENTS:") {
		t.Errorf("empty args produced an ARGUMENTS line:\n%s", out)
	}
}

func TestSkillUnknownNameListsTheRealOnes(t *testing.T) {
	tool := NewSkill(testSkills())
	_, err := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "nope"}))
	if err == nil {
		t.Fatal("unknown skill was accepted")
	}
	if !strings.Contains(err.Error(), "alpha") || !strings.Contains(err.Error(), "beta") {
		t.Errorf("error does not list the available skills: %v", err)
	}
}

func TestSkillSchemaEnumeratesSlugs(t *testing.T) {
	tool := NewSkill(testSkills())
	var schema struct {
		Properties struct {
			Name struct {
				Enum []string `json:"enum"`
			} `json:"name"`
		} `json:"properties"`
	}
	if err := json.Unmarshal(tool.InputSchema(), &schema); err != nil {
		t.Fatal(err)
	}
	if strings.Join(schema.Properties.Name.Enum, ",") != "alpha,beta" {
		t.Errorf("enum = %v, want [alpha beta]", schema.Properties.Name.Enum)
	}
}

func TestSkillDescriptionListsSkills(t *testing.T) {
	tool := NewSkill(testSkills())
	d := tool.Description()
	if !strings.Contains(d, "alpha: the first one") || !strings.Contains(d, "beta: the second one") {
		t.Errorf("tool description does not list the skills:\n%s", d)
	}
}

func TestNewSkillWithNoneIsNil(t *testing.T) {
	if NewSkill(nil) != nil {
		t.Error("NewSkill(nil) must return nil so the caller registers nothing")
	}
}

func TestSkillRepeatInvocationReturnsBodyAgain(t *testing.T) {
	// Deduping would save tokens and cost the model the ability to re-read
	// instructions that compaction dropped.
	tool := NewSkill(testSkills())
	first, _ := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "alpha"}))
	second, _ := tool.Execute(context.Background(), jsonArgs(t, map[string]any{"name": "alpha"}))
	if first != second || first == "" {
		t.Error("the second invocation did not return the same body")
	}
}
```

Add to `internal/approval/policy_test.go`:

```go
func TestSkillRunsUnattended(t *testing.T) {
	p := New(func(Request) Decision { t.Fatal("skill should not prompt"); return Deny })
	if d := p.Decide(Request{Tool: "skill"}); d != Allow {
		t.Errorf("skill decided %v, want Allow", d)
	}
}
```

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.

- [ ] **Step 2: Run the tests to verify they fail**

Run: `go test ./internal/tools/ -run Skill -v && go test ./internal/approval/ -run Skill -v`
Expected: FAIL — `undefined: NewSkill`, and the approval test fails because `skill` is not in the auto-approve set.

- [ ] **Step 3: Write the tool**

Create `internal/tools/skill.go`:

```go
package tools

import (
	"context"
	"encoding/json"
	"fmt"
	"sort"
	"strings"

	"harness/internal/skills"
)

// SkillArgs is the skill input. The name is constrained to the discovered
// slugs by an enum patched into the schema, so a hallucinated name is a schema
// violation the model sees at once rather than a lookup failure it has to
// reason about.
type SkillArgs struct {
	Name string `json:"name" jsonschema:"description=Which skill to load"`
	Args string `json:"args,omitempty" jsonschema:"description=Arguments for the skill; whatever followed the skill name"`
}

// NewSkill builds the skill tool over the skills discovered at startup. It
// returns nil when there are none, so the caller registers nothing and the
// prompt is byte-identical to a harness without the feature.
func NewSkill(found []skills.Skill) Tool {
	if len(found) == 0 {
		return nil
	}
	byName := make(map[string]skills.Skill, len(found))
	names := make([]string, 0, len(found))
	var listing strings.Builder
	for _, s := range found {
		byName[s.Name] = s
		names = append(names, s.Name)
		fmt.Fprintf(&listing, "\n  %s: %s", s.Name, s.Description)
	}
	sort.Strings(names)

	desc := strings.TrimSpace(`
Load a skill: a set of instructions for one kind of work. Invoke a skill when
its description matches what you are about to do — before starting, not after.
The instructions come back as the tool result; follow them.

Available skills:`) + listing.String()

	t := New("skill", desc, func(_ context.Context, in SkillArgs) (string, error) {
		s, ok := byName[in.Name]
		if !ok {
			return "", fmt.Errorf("no skill named %q; available: %s", in.Name, strings.Join(names, ", "))
		}
		var b strings.Builder
		fmt.Fprintf(&b, "Skill: %s\nDirectory: %s\n", s.Name, s.Dir)
		b.WriteString("Relative paths below resolve against that directory.\n\n")
		b.WriteString(s.Body)
		if a := strings.TrimSpace(in.Args); a != "" {
			b.WriteString("\n\nARGUMENTS: " + a)
		}
		return b.String(), nil
	})
	return withEnum(t, "name", names)
}

// withEnum patches an enum onto one string property of a reflected schema.
// invopop/jsonschema derives the schema from struct tags, which cannot carry a
// list only known at startup.
func withEnum(t Tool, property string, values []string) Tool {
	var m map[string]any
	if err := json.Unmarshal(t.InputSchema(), &m); err != nil {
		return t
	}
	props, ok := m["properties"].(map[string]any)
	if !ok {
		return t
	}
	p, ok := props[property].(map[string]any)
	if !ok {
		return t
	}
	p["enum"] = values
	patched, err := json.Marshal(m)
	if err != nil {
		return t
	}
	return patchedSchema{Tool: t, schema: patched}
}

// patchedSchema wraps a Tool to override only its input schema.
type patchedSchema struct {
	Tool
	schema json.RawMessage
}

func (p patchedSchema) InputSchema() json.RawMessage { return p.schema }
```

- [ ] **Step 4: Add `skill` to the auto-approved set**

In `internal/approval/policy.go`, the map at lines 40-42 becomes:

```go
	"read_file": true,
	"glob":      true,
	"grep":      true,
	// skill reads one file from a directory found at startup, and cannot
	// reach anything read_file could not already reach.
	"skill": true,
```

- [ ] **Step 5: Run the tests to verify they pass**

Run: `go test ./internal/tools/ ./internal/approval/ -v`
Expected: PASS.

- [ ] **Step 6: Commit**

```bash
cd /home/joni/Work/GO/coding-harness
gofmt -w internal/tools internal/approval
git add internal/tools internal/approval
git commit -m "tools: a skill tool, auto-approved, with a slug enum"
```

---

### Task 4: Tell the model skills exist

**Files:**
- Modify: `internal/agent/loop.go` (`Options` at line 42, `New` at line 112)
- Test: `internal/agent/loop_test.go`

**Interfaces:**
- Consumes: `skills.Skill` from Task 1.
- 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.

- [ ] **Step 1: Write the failing tests**

Add to `internal/agent/loop_test.go`:

```go
func TestSystemPromptListsSkills(t *testing.T) {
	a := New(nil, tools.NewRegistry(nil), NewState(), nil, Options{
		Skills: []skills.Skill{{Name: "commit-helper", Description: "Use when writing commit messages"}},
	})
	sys := a.opts.System
	if !strings.Contains(sys, "commit-helper: Use when writing commit messages") {
		t.Errorf("system prompt does not list the skill:\n%s", sys)
	}
	if !strings.Contains(sys, "/<name>") {
		t.Error("system prompt does not explain what a leading slash means")
	}
	if !strings.HasPrefix(sys, defaultSystem) {
		t.Error("the listing replaced the default prompt instead of being appended to it")
	}
}

func TestSystemPromptUnchangedWithoutSkills(t *testing.T) {
	a := New(nil, tools.NewRegistry(nil), NewState(), nil, Options{})
	if a.opts.System != defaultSystem {
		t.Errorf("system prompt differs from the default with no skills:\n%s", a.opts.System)
	}
}

func TestSystemPromptIsStableAcrossRequests(t *testing.T) {
	// The listing sits in the cached prefix. If it varied per request, every
	// turn would miss the cache.
	a := New(nil, tools.NewRegistry(nil), NewState(), nil, Options{
		Skills: []skills.Skill{{Name: "a", Description: "one"}, {Name: "b", Description: "two"}},
	})
	a.state.Append(llm.Message{Role: llm.RoleUser, Content: []llm.ContentBlock{llm.TextBlock("hi")}})
	first := a.request().System[0].Text
	second := a.request().System[0].Text
	if first != second {
		t.Error("the system prompt changed between two requests in one session")
	}
}
```

Add `"harness/internal/skills"` and `"strings"` to the test file's imports if they are not already there.

- [ ] **Step 2: Run the tests to verify they fail**

Run: `go test ./internal/agent/ -run SystemPrompt -v`
Expected: FAIL to compile — `unknown field Skills in struct literal`.

- [ ] **Step 3: Add the option and compose the prompt**

In `internal/agent/loop.go`, add to `Options`:

```go
	// Skills are the instruction sets discovered at startup. Their names and
	// descriptions are appended to the system prompt; the bodies are loaded on
	// demand by the skill tool.
	Skills []skills.Skill
```

Add `"harness/internal/skills"` and `"strings"` to the imports, and in `New`, right after the existing `if opts.System == "" { opts.System = defaultSystem }`:

```go
	opts.System += skillListing(opts.Skills)
```

Then add, next to `defaultSystem`:

```go
// skillListing is appended to the system prompt when skills were discovered.
// It is constant for the session, so the cached prefix still matches request to
// request: what breaks caching is content that varies per request, not content
// that varies per session.
func skillListing(found []skills.Skill) string {
	if len(found) == 0 {
		return ""
	}
	var b strings.Builder
	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:")
	for _, s := range found {
		fmt.Fprintf(&b, "\n  %s: %s", s.Name, s.Description)
	}
	return b.String()
}
```

`fmt` is already imported by `loop.go`; confirm before adding it again.

- [ ] **Step 4: Run the tests to verify they pass**

Run: `go test ./internal/agent/ -v`
Expected: PASS, including the existing compaction and loop tests.

- [ ] **Step 5: Commit**

```bash
cd /home/joni/Work/GO/coding-harness
gofmt -w internal/agent
git add internal/agent
git commit -m "agent: list discovered skills in the cached system prompt"
```

---

### Task 5: `/skills` and `/slug` in the TUI

**Files:**
- Modify: `internal/tui/model.go` (`submit` at line 426, `helpText` at line 546, `Options` struct)
- Test: `internal/tui/model_test.go`

**Interfaces:**
- Consumes: `skills.Skill` from Task 1.
- Produces: a `Skills []skills.Skill` field on the TUI's options struct. Task 6 sets it.

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.

- [ ] **Step 1: Write the failing tests**

Add to `internal/tui/model_test.go`, following the shape of the existing `/help` test at line 231:

```go
func TestSlashSkillsListsThem(t *testing.T) {
	m := newTestModel(t)
	m.opts.Skills = []skills.Skill{
		{Name: "commit-helper", Description: "Use when writing commit messages", Source: "personal"},
	}
	m.submit("/skills")
	out := m.tr.String()
	if !strings.Contains(out, "commit-helper") || !strings.Contains(out, "Use when writing commit messages") {
		t.Errorf("/skills did not list the skill:\n%s", out)
	}
}

func TestSlashSkillsWithNoneSaysSo(t *testing.T) {
	m := newTestModel(t)
	m.submit("/skills")
	if !strings.Contains(m.tr.String(), "no skills") {
		t.Errorf("/skills with none found said nothing useful:\n%s", m.tr.String())
	}
}

func TestKnownSkillIsSentAsAUserTurn(t *testing.T) {
	m := newTestModel(t)
	m.opts.Skills = []skills.Skill{{Name: "commit-helper", Description: "d"}}
	m.submit("/commit-helper fix the flaky test")
	if m.phase != phaseRunning {
		t.Errorf("phase = %v, want phaseRunning — the line was not sent", m.phase)
	}
	if !strings.Contains(m.tr.String(), "/commit-helper fix the flaky test") {
		t.Errorf("the user turn is not in the transcript:\n%s", m.tr.String())
	}
}

func TestUnknownSlashCommandStillErrors(t *testing.T) {
	m := newTestModel(t)
	m.opts.Skills = []skills.Skill{{Name: "commit-helper", Description: "d"}}
	m.submit("/commit-helpr")
	if m.phase == phaseRunning {
		t.Error("a typo was sent to the model instead of being caught")
	}
	if !strings.Contains(m.tr.String(), "unknown command") {
		t.Errorf("no error for the typo:\n%s", m.tr.String())
	}
}

func TestHelpMentionsSkills(t *testing.T) {
	m := newTestModel(t)
	m.submit("/help")
	if !strings.Contains(m.tr.String(), "/skills") {
		t.Error("/help does not mention /skills")
	}
}
```

`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.

- [ ] **Step 2: Run the tests to verify they fail**

Run: `go test ./internal/tui/ -run 'Skill|Help' -v`
Expected: FAIL — `m.opts.Skills` undefined.

- [ ] **Step 3: Add the field and the two cases**

Add to the TUI options struct:

```go
	// Skills are the instruction sets discovered at startup, used to resolve
	// /<slug> and to answer /skills.
	Skills []skills.Skill
```

In `submit`, add the `/skills` case anywhere among the existing command cases:

```go
	case line == "/skills":
		if len(m.opts.Skills) == 0 {
			m.tr.addNotice("no skills found in ~/" + skills.Dirname + " or ./" + skills.Dirname)
		} else {
			var b strings.Builder
			b.WriteString("skills:")
			for _, s := range m.opts.Skills {
				fmt.Fprintf(&b, "\n  /%s  (%s) \u2014 %s", s.Name, s.Source, s.Description)
			}
			m.tr.addNotice(b.String())
		}
		return m, m.sync()
```

Then make one change to the existing unknown-command case at line 448 — add the
guard, and nothing else:

```go
	case strings.HasPrefix(line, "/") && !m.isSkill(line):
		m.tr.addError("unknown command " + line + " — /help lists them")
		return m, m.sync()
	}
```

A recognised skill now matches no case, falls out of the switch, and reaches
the existing user-turn code at the bottom of `submit` untouched. The model sees
the slash, the system prompt explains what it means, and the skill tool does
the loading — one code path for both ways in.

Add the helper next to `submit`:

```go
// isSkill reports whether a slash command names a discovered skill. Matching
// here rather than sending every unrecognised slash to the model means a typo
// is caught by the harness instead of puzzled over by the agent.
func (m *model) isSkill(line string) bool {
	name := strings.TrimPrefix(line, "/")
	if i := strings.IndexAny(name, " \t"); i >= 0 {
		name = name[:i]
	}
	for _, s := range m.opts.Skills {
		if s.Name == name {
			return true
		}
	}
	return false
}
```

Update `helpText` at line 546:

```go
const helpText = `commands: /allowed  /clear  /help  /skills  /exit
```

Keep the rest of that const exactly as it is. Add `"harness/internal/skills"` and `"fmt"` to the imports if absent.

- [ ] **Step 4: Run the tests to verify they pass**

Run: `go test ./internal/tui/ -v`
Expected: PASS, including the existing look and colour tests.

- [ ] **Step 5: Commit**

```bash
cd /home/joni/Work/GO/coding-harness
gofmt -w internal/tui
git add internal/tui
git commit -m "tui: /skills, and /<slug> as a user turn"
```

---

### Task 6: Wire it up, and document it

**Files:**
- Modify: `cmd/harness/main.go` (flags near line 55, wiring at lines 101-130)
- Modify: `README.md`
- Test: manual, per the steps below

**Interfaces:**
- Consumes: everything from Tasks 1-5.
- Produces: the working feature. Nothing depends on this task.

- [ ] **Step 1: Add the flag**

In `cmd/harness/main.go`, after the `-no-tui` flag at line 55:

```go
	noSkills := flag.Bool("no-skills", false, "skip discovering skills in ~/"+skills.Dirname+" and ./"+skills.Dirname)
```

- [ ] **Step 2: Discover, and wire the results through**

Replace the registry construction at lines 101-109:

```go
	var found []skills.Skill
	if !*noSkills {
		home, err := os.UserHomeDir()
		if err != nil {
			// No home directory is not fatal: project skills still work.
			home = ""
		}
		var problems []error
		found, problems = skills.Discover(home, root)
		for _, p := range problems {
			notice("skill skipped: %v", p)
		}
	}

	// Skill directories are readable by the search tools and by nothing else,
	// so a skill can read the files it ships with.
	var skillDirs []string
	for _, s := range found {
		skillDirs = append(skillDirs, s.Dir)
	}

	state := agent.NewState()
	reg := tools.NewRegistry(log).Register(
		tools.NewRead(root, state, skillDirs...),
		tools.NewGlob(root, skillDirs...),
		tools.NewGrep(root, skillDirs...),
		tools.NewEdit(root, state),
		tools.NewWrite(root, state),
		tools.NewBash(root),
	)
	if t := tools.NewSkill(found); t != nil {
		reg.Register(t)
	}
```

Add `Skills: found,` to the `agent.Options` literal at line 123.

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.

Confirm `"harness/internal/skills"` and `"os"` are imported.

- [ ] **Step 3: Build and run the whole suite**

Run: `go build ./... && go test ./...`
Expected: PASS.

- [ ] **Step 4: Verify the feature by hand against the mock API**

```bash
cd /home/joni/Work/GO/coding-harness
mkdir -p /tmp/skilldemo/.agents/skills/demo-skill/references
cat > /tmp/skilldemo/.agents/skills/demo-skill/SKILL.md <<'EOF'
---
name: demo-skill
description: Use when demonstrating that skills load
---
Say the word BANANA, then read references/note.md and say what it contains.
EOF
echo "the note says pineapple" > /tmp/skilldemo/.agents/skills/demo-skill/references/note.md
git -C /tmp/skilldemo init -q
go build -o harness ./cmd/harness
./harness -C /tmp/skilldemo -no-tui -no-skills -p "/demo-skill" 2>&1 | head -5
```

Expected with `-no-skills`: the model is told nothing about skills and has no tool, so it treats `/demo-skill` as ordinary text.

Then, with a real key and without the flag:

```bash
./harness -C /tmp/skilldemo -no-tui -p "/demo-skill"
```

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.

- [ ] **Step 5: Verify `/skills` in the TUI**

```bash
./harness -C /tmp/skilldemo
```

Type `/skills` — it must list `demo-skill (project)`. Type `/demo-skil` — it must say `unknown command`. Type `/help` — it must list `/skills`.

- [ ] **Step 6: Document it in the README**

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`:

````markdown
## Skills

A skill is a directory of instructions the agent pulls in when it is relevant:

```
~/.agents/skills/commit-helper/SKILL.md      every repo
./.agents/skills/commit-helper/SKILL.md      this one, checked in
```

```markdown
---
name: commit-helper
description: Use when writing commit messages for this repo
---

Read the last twenty commits for tone before writing anything.
Conventions are in references/conventions.md.
```

The directory name is the skill's name, and a `name:` that disagrees with it is
a load error. Only the description reaches the model until the skill is
invoked, so it is what decides whether the skill fires at all; the body can be
as long as you like.

Two ways in, one way to load. The agent calls the `skill` tool when a
description matches what it is doing; you can force one with `/commit-helper
fix the flaky test`, which the model answers by making the same call. `/skills`
lists what was found, project shadowing personal on a name collision.

Skill directories are readable by `read_file`, `glob` and `grep` even though
they sit outside the working root, so a skill can ship reference files and
point at them. They are not writable by anything: `edit_file`, `write_file` and
`bash` keep the working root as their only root. A skill is input, not a work
product.

Discovery happens at startup. The name-and-description listing goes in the
cached prompt prefix, so re-reading the directory each turn would cost a cache
miss on every turn for a file that never changes. Restart to pick up a new
skill. `-no-skills` turns the whole thing off; the eval runner never discovers
skills at all, so METRICS.md is unaffected by what is in your home directory.
````

Add `internal/skills   discovery and parsing of SKILL.md` to the layout block near the end of the README.

- [ ] **Step 7: Commit**

```bash
cd /home/joni/Work/GO/coding-harness
gofmt -w ./cmd ./internal
go build ./... && go test ./...
git add cmd README.md
git commit -m "harness: discover and wire up skills, document them"
```

---

## Self-Review

**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.

**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.

**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.