johannSoCodeyPublic

skills: stop the skill preamble promising what read_file does not do

The tool result said "Relative paths below resolve against that
directory", but read_file anchors a relative path to the working root.
A model that believed the line asked for references/note.md, got a bare
"no such file", and had nothing to work back from — verified end to end
against a fake API driving the real tool stack.

Two halves. The preamble now says to join relative paths onto the
absolute directory it prints just above. And read_file's not-found error
names the skill directories when it has any, so a model that ignores the
preamble can still recover on the next turn; with no skills there are no
extra roots and the message stays as terse as it was.

The wrong sentence originated in the spec, so the spec is amended too,
with the reasoning that was missing from it.

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

Browse Files

Jjonicommitted 4 weeks ago1 parent b755ae9commitf04f191

Showing 7 changed files with 85 additions and 4 deletions.

+6−0README.mdView File

@@ -160,6 +160,12 @@ point at them. They are not writable by anything: `edit_file`, `write_file` and
160	160	`bash` keep the working root as their only root. A skill is input, not a work
161	161	product.
162	162
	163	+ Relative paths still anchor to the working root, so a skill pointing at its own
	164	+ files is read with the directory the `skill` tool prints joined on. If the model
	165	+ forgets, the "no such file" it gets back names the skill directories, which is
	166	+ usually enough to recover on the next turn. A shadowed skill's directory is not
	167	+ readable — only the skills actually in play widen anything.
	168	+
163	169	Discovery happens at startup. The name-and-description listing goes in the
164	170	cached prompt prefix, so re-reading the directory each turn would cost a cache
165	171	miss on every turn for a file that never changes. Restart to pick up a new

+1−1docs/superpowers/plans/2026-09-17-agents-skills.mdView File

@@ -888,7 +888,7 @@ Available skills:`) + listing.String()
888	888	}
889	889	var b strings.Builder
890	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")
	891	+ b.WriteString("Join relative paths onto the directory above before reading them.\n\n")
892	892	b.WriteString(s.Body)
893	893	if a := strings.TrimSpace(in.Args); a != "" {
894	894	b.WriteString("\n\nARGUMENTS: " + a)

+14−1docs/superpowers/specs/2026-09-17-agents-skills-design.mdView File

@@ -84,11 +84,24 @@ It returns the body with a preamble naming the skill's directory:
84	84	```
85	85	Skill: commit-helper
86	86	Directory: /home/joni/.agents/skills/commit-helper
87		- Relative paths below resolve against that directory.
	87	+ Join relative paths onto the directory above before reading them.
88	88
89	89	<body>
90	90	```
91	91
	92	+ The wording matters. `read_file` anchors a relative path to the working
	93	+ root, not to the skill directory, so a preamble promising that relative
	94	+ paths "resolve against that directory" is a lie the model will act on: it
	95	+ asks for `references/conventions.md`, gets a bare "no such file", and has
	96	+ nothing to work back from. The directory is given absolutely and the model
	97	+ is told to join onto it.
	98	+
	99	+ `read_file`'s not-found error carries the other half. When the tool has
	100	+ extra roots — which it only does when skills were discovered — a miss on a
	101	+ relative path names them, so a model that ignores the preamble still learns
	102	+ where the file it wants actually lives. With no skills there are no extra
	103	+ roots and the message stays exactly as terse as it was.
	104	+
92	105	`args`, when present, is appended under an `ARGUMENTS:` line. The tool is
93	106	read-only and runs unattended like `read_file` — it reads one file from a
94	107	directory discovered at startup, and cannot reach anything the read tools

+28−0internal/tools/jail_test.goView File

@@ -170,3 +170,31 @@ func TestToolsRefuseToEscapeTheRoot(t *testing.T) {
170	170	t.Error("the file outside the root was modified")
171	171	}
172	172	}
	173	+
	174	+ // A model that follows a skill's own wording and asks for "references/note.md"
	175	+ // gets a miss, because relative paths anchor to the working root. The error is
	176	+ // the only place it can learn that skill directories exist at all.
	177	+ func TestReadNotFoundNamesSkillDirsWhenThereAreSome(t *testing.T) {
	178	+ root := t.TempDir()
	179	+ skill := t.TempDir()
	180	+
	181	+ withSkills := NewRead(root, nil, skill)
	182	+ _, err := run(t, withSkills, map[string]any{"path": "references/note.md"})
	183	+ if err == nil {
	184	+ t.Fatal("reading a missing file succeeded")
	185	+ }
	186	+ if !strings.Contains(err.Error(), skill) {
	187	+ t.Errorf("error does not name the skill directory, so the model cannot recover:\n%v", err)
	188	+ }
	189	+
	190	+ // Without extra roots there are no skill directories to mention, and the
	191	+ // message must stay exactly as terse as it has always been.
	192	+ plain := NewRead(root, nil)
	193	+ _, err = run(t, plain, map[string]any{"path": "references/note.md"})
	194	+ if err == nil {
	195	+ t.Fatal("reading a missing file succeeded")
	196	+ }
	197	+ if got, want := err.Error(), "no such file: references/note.md"; got != want {
	198	+ t.Errorf("error without skills changed\n got: %s\nwant: %s", got, want)
	199	+ }
	200	+ }

+14−1internal/tools/read.goView File

@@ -55,7 +55,7 @@ them in text you write back to the file.`),
55	55	info, err := os.Stat(path)
56	56	switch {
57	57	case os.IsNotExist(err):
58		- return "", fmt.Errorf("no such file: %s", in.Path)
	58	+ return "", notFound(in.Path, extra)
59	59	case err != nil:
60	60	return "", err
61	61	case info.IsDir():
@@ -112,6 +112,19 @@ func splitLines(s string) []string {
112	112	return lines
113	113	}
114	114
	115	+ // notFound reports a missing file, naming the skill directories when there are
	116	+ // any. A relative path anchors to the working root, so a model following a
	117	+ // skill's own wording asks for "references/note.md" and misses; this error is
	118	+ // the only place it can learn where that file actually lives.
	119	+ func notFound(path string, extra []string) error {
	120	+ if len(extra) == 0 \|\| filepath.IsAbs(path) {
	121	+ return fmt.Errorf("no such file: %s", path)
	122	+ }
	123	+ return fmt.Errorf("no such file: %s — relative paths are resolved against the working root. "+
	124	+ "If this belongs to a skill, join it onto that skill's directory: %s",
	125	+ path, strings.Join(extra, ", "))
	126	+ }
	127	+
115	128	// resolve anchors a relative path to root and refuses anything outside it.
116	129	func resolve(root, path string) (string, error) {
117	130	return resolveWith(root, nil, path)

+4−1internal/tools/skill.goView File

@@ -50,7 +50,10 @@ Available skills:`) + listing.String()
50	50	}
51	51	var b strings.Builder
52	52	fmt.Fprintf(&b, "Skill: %s\nDirectory: %s\n", s.Name, s.Dir)
53		- b.WriteString("Relative paths below resolve against that directory.\n\n")
	53	+ // Not "relative paths resolve against that directory" — read_file
	54	+ // anchors a relative path to the working root, so promising
	55	+ // otherwise sends the model into a "no such file" it cannot diagnose.
	56	+ b.WriteString("Join relative paths onto the directory above before reading them.\n\n")
54	57	b.WriteString(s.Body)
55	58	if a := strings.TrimSpace(in.Args); a != "" {
56	59	b.WriteString("\n\nARGUMENTS: " + a)

+18−0internal/tools/skill_test.goView File

@@ -101,3 +101,21 @@ func TestSkillRepeatInvocationReturnsBodyAgain(t *testing.T) {
101	101	t.Error("the second invocation did not return the same body")
102	102	}
103	103	}
	104	+
	105	+ // The preamble tells the model how to reach a skill's bundled files. It has to
	106	+ // be true: read_file resolves a relative path against the working root, not
	107	+ // against the skill directory, so an instruction to use relative paths sends
	108	+ // the model straight into a "no such file" it cannot diagnose.
	109	+ func TestSkillPreambleDoesNotPromiseRelativeResolution(t *testing.T) {
	110	+ tool := NewSkill(testSkills())
	111	+ out, err := run(t, tool, map[string]any{"name": "alpha"})
	112	+ if err != nil {
	113	+ t.Fatal(err)
	114	+ }
	115	+ if strings.Contains(out, "resolve against that directory") {
	116	+ t.Errorf("preamble still claims relative paths resolve against the skill directory:\n%s", out)
	117	+ }
	118	+ if !strings.Contains(out, "Join relative paths onto the directory above") {
	119	+ t.Errorf("preamble does not tell the model to join onto the directory:\n%s", out)
	120	+ }
	121	+ }