diff --git a/docs/contributing/runtime-implementation.md b/docs/contributing/runtime-implementation.md index 74a9da9e10..26effc489d 100644 --- a/docs/contributing/runtime-implementation.md +++ b/docs/contributing/runtime-implementation.md @@ -319,7 +319,7 @@ slots: │ Personal: /sandbox/claude-config/skills/ (fullsend) │ │ Project: /.claude/skills/ (repo) │ │ Precedence: personal > project (name collision → │ -│ fullsend wins, repo version shadowed) │ +│ fullsend wins, repo shadowed with warning)│ │ Repo skills extend the agent; use config-driven │ │ agent registration for org-level skill overrides │ └────────────────────────────────────────────────────────┘ diff --git a/docs/guides/user/customizing-agents.md b/docs/guides/user/customizing-agents.md index 38c828d1b9..ffaa330438 100644 --- a/docs/guides/user/customizing-agents.md +++ b/docs/guides/user/customizing-agents.md @@ -156,7 +156,8 @@ are actually changing: [fullsend-ai/agents](https://github.com/fullsend-ai/agents). Do not guess field names or roster lists from memory. 2. **Unique skill names** — a repo skill with the same directory name as a - built-in is ignored (see [skill precedence](customizing-with-skills.md#skill-precedence)). + built-in is ignored and produces a warning (see + [skill precedence](customizing-with-skills.md#skill-precedence)). 3. **Specificity wins** — vague augmentations lose to hard default instructions. Own exact fields; use word limits and templates. 4. **Sub-agents are not wrapper skills** — if you need a new review dimension, diff --git a/docs/guides/user/customizing-with-skills.md b/docs/guides/user/customizing-with-skills.md index 29d7224577..06f7b16274 100644 --- a/docs/guides/user/customizing-with-skills.md +++ b/docs/guides/user/customizing-with-skills.md @@ -87,8 +87,9 @@ architecture constraints — without modifying any fullsend configuration. Repo skills **extend** the agent's skill set. They do not replace built-in skills. If a repo skill has the same name as a built-in skill, the built-in -version takes precedence and the repo version is silently ignored. Use a -unique name to ensure your skill is discoverable. +version takes precedence and the repo version is ignored. Fullsend warns about +the collision before the agent starts. Use a unique name to extend the agent, +or intentionally replace it through [`base:` harness composition](#overriding-built-in-skills). ### Skill precedence @@ -104,7 +105,8 @@ Personal (CLAUDE_CONFIG_DIR/skills/) > Project (.claude/skills/) A repo skill with a novel name (no collision) is always available. A repo skill with a name matching a built-in skill is shadowed — the agent never -sees it. +sees it. Fullsend logs a warning naming the shadowed skill and the supported +extension and override paths. ### Extension points diff --git a/internal/cli/run.go b/internal/cli/run.go index 975eb8442b..db099a9416 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -1643,6 +1643,9 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep } } boot := newHarnessBootstrap(h, sandboxName, agentName, forgeEgressEntry) + if rt.Name() == "claude" { + warnRepoSkillCollisions(hostRepositoryDir, boot.SkillDirs(), printer) + } if h.SecurityEnabled() { // Scan all runtime content before upload so warnings surface together. // Host files could change between scan and upload; the runner owns the host FS here. diff --git a/internal/cli/skill_collision.go b/internal/cli/skill_collision.go new file mode 100644 index 0000000000..5eafe83e30 --- /dev/null +++ b/internal/cli/skill_collision.go @@ -0,0 +1,57 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/fullsend-ai/fullsend/internal/ui" +) + +// warnRepoSkillCollisions reports repo skills that Claude Code will ignore +// because harness skills are installed at the higher-precedence personal level. +func warnRepoSkillCollisions(repoDir string, harnessSkillDirs []string, printer *ui.Printer) { + harnessSkills := make(map[string]struct{}, len(harnessSkillDirs)) + for _, skillDir := range harnessSkillDirs { + if skillDir == "" { + continue + } + if isReadableSkillMarker(skillDir) { + harnessSkills[filepath.Base(skillDir)] = struct{}{} + } + } + + projectSkillsDir := filepath.Join(repoDir, ".claude", "skills") + entries, err := os.ReadDir(projectSkillsDir) + if err != nil { + return + } + + for _, entry := range entries { + name := entry.Name() + if _, collision := harnessSkills[name]; !collision { + continue + } + if !isReadableSkillMarker(filepath.Join(projectSkillsDir, name)) { + continue + } + printer.StepWarn(fmt.Sprintf( + "Repo skill %q is shadowed by a harness skill of the same name; use a unique skill name to extend it, or use base: harness composition to override it", + name, + )) + } +} + +func isReadableSkillMarker(skillDir string) bool { + marker := filepath.Join(skillDir, "SKILL.md") + info, err := os.Stat(marker) + if err != nil || !info.Mode().IsRegular() { + return false + } + file, err := os.Open(marker) + if err != nil { + return false + } + _ = file.Close() + return true +} diff --git a/internal/cli/skill_collision_test.go b/internal/cli/skill_collision_test.go new file mode 100644 index 0000000000..bbbcaf4d15 --- /dev/null +++ b/internal/cli/skill_collision_test.go @@ -0,0 +1,107 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/ui" +) + +func TestWarnRepoSkillCollisions_WarnsForShadowedSkill(t *testing.T) { + repoDir := t.TempDir() + repoSkillDir := filepath.Join(repoDir, ".claude", "skills", "code-review") + require.NoError(t, os.MkdirAll(repoSkillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(repoSkillDir, "SKILL.md"), []byte("# Repo review"), 0o644)) + + harnessSkillDir := filepath.Join(t.TempDir(), "code-review") + require.NoError(t, os.MkdirAll(harnessSkillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(harnessSkillDir, "SKILL.md"), []byte("# Harness review"), 0o644)) + + var output bytes.Buffer + warnRepoSkillCollisions(repoDir, []string{harnessSkillDir}, ui.New(&output)) + + assert.Contains(t, output.String(), `Repo skill "code-review" is shadowed by a harness skill of the same name`) + assert.Contains(t, output.String(), "use a unique skill name to extend it") + assert.Contains(t, output.String(), "base: harness composition to override it") +} + +func TestWarnRepoSkillCollisions_DoesNotWarnWithoutCollision(t *testing.T) { + repoDir := t.TempDir() + projectSkillsDir := filepath.Join(repoDir, ".claude", "skills") + require.NoError(t, os.MkdirAll(filepath.Join(projectSkillsDir, "repo-only"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectSkillsDir, "repo-only", "SKILL.md"), []byte("# Repo only"), 0o644)) + // A matching harness directory without SKILL.md is not a discoverable skill. + require.NoError(t, os.MkdirAll(filepath.Join(projectSkillsDir, "markerless-harness"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectSkillsDir, "markerless-harness", "SKILL.md"), []byte("# Repo skill"), 0o644)) + // A matching repository directory without SKILL.md is not a discoverable skill. + require.NoError(t, os.MkdirAll(filepath.Join(projectSkillsDir, "markerless-repo"), 0o755)) + // SKILL.md must be a regular file on both sides. + require.NoError(t, os.MkdirAll(filepath.Join(projectSkillsDir, "directory-marker-harness"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectSkillsDir, "directory-marker-harness", "SKILL.md"), []byte("# Repo skill"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(projectSkillsDir, "directory-marker-repo", "SKILL.md"), 0o755)) + + harnessRoot := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(harnessRoot, "markerless-harness"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(harnessRoot, "markerless-repo"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(harnessRoot, "markerless-repo", "SKILL.md"), []byte("# Harness skill"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(harnessRoot, "directory-marker-harness", "SKILL.md"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(harnessRoot, "directory-marker-repo"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(harnessRoot, "directory-marker-repo", "SKILL.md"), []byte("# Harness skill"), 0o644)) + harnessSkills := []string{ + "", + filepath.Join(harnessRoot, "harness-only"), + filepath.Join(harnessRoot, "markerless-harness"), + filepath.Join(harnessRoot, "markerless-repo"), + filepath.Join(harnessRoot, "directory-marker-harness"), + filepath.Join(harnessRoot, "directory-marker-repo"), + } + + var output bytes.Buffer + warnRepoSkillCollisions(repoDir, harnessSkills, ui.New(&output)) + + assert.Empty(t, output.String()) +} + +func TestWarnRepoSkillCollisions_DoesNotWarnForUnreadableSkillMarker(t *testing.T) { + for _, unreadableSide := range []string{"harness", "repo"} { + t.Run(unreadableSide, func(t *testing.T) { + repoDir := t.TempDir() + repoSkillDir := filepath.Join(repoDir, ".claude", "skills", "code-review") + require.NoError(t, os.MkdirAll(repoSkillDir, 0o755)) + repoMarker := filepath.Join(repoSkillDir, "SKILL.md") + require.NoError(t, os.WriteFile(repoMarker, []byte("# Repo review"), 0o644)) + + harnessSkillDir := filepath.Join(t.TempDir(), "code-review") + require.NoError(t, os.MkdirAll(harnessSkillDir, 0o755)) + harnessMarker := filepath.Join(harnessSkillDir, "SKILL.md") + require.NoError(t, os.WriteFile(harnessMarker, []byte("# Harness review"), 0o644)) + + unreadableMarker := harnessMarker + if unreadableSide == "repo" { + unreadableMarker = repoMarker + } + require.NoError(t, os.Chmod(unreadableMarker, 0o000)) + if file, err := os.Open(unreadableMarker); err == nil { + require.NoError(t, file.Close()) + t.Skip("filesystem does not enforce unreadable file permissions") + } + + var output bytes.Buffer + warnRepoSkillCollisions(repoDir, []string{harnessSkillDir}, ui.New(&output)) + + assert.Empty(t, output.String()) + }) + } +} + +func TestWarnRepoSkillCollisions_DoesNotWarnWithoutProjectSkillsDirectory(t *testing.T) { + var output bytes.Buffer + warnRepoSkillCollisions(t.TempDir(), nil, ui.New(&output)) + + assert.Empty(t, output.String()) +} diff --git a/skills/author-fullsend-augmentations/SKILL.md b/skills/author-fullsend-augmentations/SKILL.md index 2d029b9c17..515ec525a9 100644 --- a/skills/author-fullsend-augmentations/SKILL.md +++ b/skills/author-fullsend-augmentations/SKILL.md @@ -112,8 +112,9 @@ Read the current wording in fullsend: **Same name as a built-in:** If your repo skill directory matches a built-in skill name (for example `retro-analysis`), the agent **never loads your -version** — the built-in wins with no error. Use a **unique directory name** -(for example `my-org-retro`, not `retro-analysis`). +version** — the built-in wins and Fullsend logs a warning. Use a **unique +directory name** (for example `my-org-retro`, not `retro-analysis`) to extend +the agent, or use `base:` harness composition for an intentional override. **Different name:** Your skill loads **next to** built-ins. To change behavior, be **more specific** than the default — exact fields, word limits, templates. @@ -509,7 +510,7 @@ Primary docs to re-read every run: | Adding a sub-agent file the parent never dispatches | Read parent roster/selection; update parent or upstream it | | User asked for sub-agent; you created `/SKILL.md` | Use `sub-agents/.md` + parent roster edits; no wrapper skill | | Redefining default procedures | Constrain outputs; don't replace steps | -| Same directory name as a built-in | Rename — built-in shadows silently | +| Same directory name as a built-in | Rename to extend, or override through `base:` harness composition; Fullsend warns that the repo skill is shadowed | | Suggesting `customized/` or overlay dirs for overrides | Use harness mechanisms from current docs (file-level when available) | | Hardcoding "org fork" as the only sub-agent path | Re-discover shipping families each run | | Writing into discovery or test cwd | Ask target repo; draft in chat until user names path |