From 55382a944409352463db5f5ef14ea41862cb49ff Mon Sep 17 00:00:00 2001 From: Shai Revivo Date: Mon, 31 Aug 2026 14:01:35 +0300 Subject: [PATCH 1/4] feat(#2661): warn when repo skills are shadowed Signed-off-by: Shai Revivo Co-authored-by: Codex --- docs/contributing/runtime-implementation.md | 2 +- docs/guides/user/customizing-agents.md | 3 +- docs/guides/user/customizing-with-skills.md | 8 +-- internal/cli/run.go | 3 ++ internal/cli/skill_collision.go | 40 +++++++++++++++ internal/cli/skill_collision_test.go | 50 +++++++++++++++++++ skills/author-fullsend-augmentations/SKILL.md | 7 +-- 7 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 internal/cli/skill_collision.go create mode 100644 internal/cli/skill_collision_test.go diff --git a/docs/contributing/runtime-implementation.md b/docs/contributing/runtime-implementation.md index 25dcc80b44..612219739e 100644 --- a/docs/contributing/runtime-implementation.md +++ b/docs/contributing/runtime-implementation.md @@ -326,7 +326,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..9b95dd6aa4 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 use a derived harness with `base:` composition for an intentional override. ### 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 78bda7a885..83e44df0d6 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -1644,6 +1644,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..c53b054662 --- /dev/null +++ b/internal/cli/skill_collision.go @@ -0,0 +1,40 @@ +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 != "" { + 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 info, statErr := os.Stat(filepath.Join(projectSkillsDir, name, "SKILL.md")); statErr != nil || !info.Mode().IsRegular() { + 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, + )) + } +} diff --git a/internal/cli/skill_collision_test.go b/internal/cli/skill_collision_test.go new file mode 100644 index 0000000000..1490e73d10 --- /dev/null +++ b/internal/cli/skill_collision_test.go @@ -0,0 +1,50 @@ +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)) + + 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 directory without SKILL.md is not a discoverable repo skill. + require.NoError(t, os.MkdirAll(filepath.Join(projectSkillsDir, "not-a-skill"), 0o755)) + + harnessRoot := t.TempDir() + harnessSkills := []string{ + filepath.Join(harnessRoot, "harness-only"), + filepath.Join(harnessRoot, "not-a-skill"), + } + + var output bytes.Buffer + warnRepoSkillCollisions(repoDir, harnessSkills, 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 | From a3552f72b3c10682db21a08f9c746b7c8d818a35 Mon Sep 17 00:00:00 2001 From: Shai Revivo Date: Mon, 31 Aug 2026 15:10:51 +0300 Subject: [PATCH 2/4] fix(#2661): validate harness skills before warning Signed-off-by: Shai Revivo --- docs/guides/user/customizing-with-skills.md | 2 +- internal/cli/skill_collision.go | 5 ++++- internal/cli/skill_collision_test.go | 11 ++++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/guides/user/customizing-with-skills.md b/docs/guides/user/customizing-with-skills.md index 9b95dd6aa4..06f7b16274 100644 --- a/docs/guides/user/customizing-with-skills.md +++ b/docs/guides/user/customizing-with-skills.md @@ -89,7 +89,7 @@ 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 ignored. Fullsend warns about the collision before the agent starts. Use a unique name to extend the agent, -or use a derived harness with `base:` composition for an intentional override. +or intentionally replace it through [`base:` harness composition](#overriding-built-in-skills). ### Skill precedence diff --git a/internal/cli/skill_collision.go b/internal/cli/skill_collision.go index c53b054662..9cfb747caa 100644 --- a/internal/cli/skill_collision.go +++ b/internal/cli/skill_collision.go @@ -13,7 +13,10 @@ import ( func warnRepoSkillCollisions(repoDir string, harnessSkillDirs []string, printer *ui.Printer) { harnessSkills := make(map[string]struct{}, len(harnessSkillDirs)) for _, skillDir := range harnessSkillDirs { - if skillDir != "" { + if skillDir == "" { + continue + } + if info, err := os.Stat(filepath.Join(skillDir, "SKILL.md")); err == nil && info.Mode().IsRegular() { harnessSkills[filepath.Base(skillDir)] = struct{}{} } } diff --git a/internal/cli/skill_collision_test.go b/internal/cli/skill_collision_test.go index 1490e73d10..1a82c29790 100644 --- a/internal/cli/skill_collision_test.go +++ b/internal/cli/skill_collision_test.go @@ -20,6 +20,7 @@ func TestWarnRepoSkillCollisions_WarnsForShadowedSkill(t *testing.T) { 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)) @@ -34,13 +35,17 @@ func TestWarnRepoSkillCollisions_DoesNotWarnWithoutCollision(t *testing.T) { 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 directory without SKILL.md is not a discoverable repo skill. - require.NoError(t, os.MkdirAll(filepath.Join(projectSkillsDir, "not-a-skill"), 0o755)) + // 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)) harnessRoot := t.TempDir() harnessSkills := []string{ filepath.Join(harnessRoot, "harness-only"), - filepath.Join(harnessRoot, "not-a-skill"), + filepath.Join(harnessRoot, "markerless-harness"), + filepath.Join(harnessRoot, "markerless-repo"), } var output bytes.Buffer From 6d42a15228d6501c0653a252a3e43607800da294 Mon Sep 17 00:00:00 2001 From: Shai Revivo Date: Mon, 31 Aug 2026 15:21:33 +0300 Subject: [PATCH 3/4] fix(#2661): ignore unreadable skill markers Signed-off-by: Shai Revivo --- internal/cli/skill_collision.go | 18 ++++++++-- internal/cli/skill_collision_test.go | 52 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/internal/cli/skill_collision.go b/internal/cli/skill_collision.go index 9cfb747caa..5eafe83e30 100644 --- a/internal/cli/skill_collision.go +++ b/internal/cli/skill_collision.go @@ -16,7 +16,7 @@ func warnRepoSkillCollisions(repoDir string, harnessSkillDirs []string, printer if skillDir == "" { continue } - if info, err := os.Stat(filepath.Join(skillDir, "SKILL.md")); err == nil && info.Mode().IsRegular() { + if isReadableSkillMarker(skillDir) { harnessSkills[filepath.Base(skillDir)] = struct{}{} } } @@ -32,7 +32,7 @@ func warnRepoSkillCollisions(repoDir string, harnessSkillDirs []string, printer if _, collision := harnessSkills[name]; !collision { continue } - if info, statErr := os.Stat(filepath.Join(projectSkillsDir, name, "SKILL.md")); statErr != nil || !info.Mode().IsRegular() { + if !isReadableSkillMarker(filepath.Join(projectSkillsDir, name)) { continue } printer.StepWarn(fmt.Sprintf( @@ -41,3 +41,17 @@ func warnRepoSkillCollisions(repoDir string, harnessSkillDirs []string, printer )) } } + +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 index 1a82c29790..bbbcaf4d15 100644 --- a/internal/cli/skill_collision_test.go +++ b/internal/cli/skill_collision_test.go @@ -40,12 +40,25 @@ func TestWarnRepoSkillCollisions_DoesNotWarnWithoutCollision(t *testing.T) { 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 @@ -53,3 +66,42 @@ func TestWarnRepoSkillCollisions_DoesNotWarnWithoutCollision(t *testing.T) { 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()) +} From 7fb9f05aa8bba0675058c6b37958eae739399462 Mon Sep 17 00:00:00 2001 From: Shai Revivo Date: Tue, 1 Sep 2026 16:39:48 +0300 Subject: [PATCH 4/4] fix(#2661): recognize alternate skill marker casing Co-authored-by: Codex Signed-off-by: Shai Revivo --- internal/cli/bootstrap_scan.go | 4 +++- internal/cli/skill_collision.go | 27 +++++++++++++++++---------- internal/cli/skill_collision_test.go | 13 +++++++++++++ 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/internal/cli/bootstrap_scan.go b/internal/cli/bootstrap_scan.go index b8ef0a0146..e371df7bc3 100644 --- a/internal/cli/bootstrap_scan.go +++ b/internal/cli/bootstrap_scan.go @@ -9,6 +9,8 @@ import ( "github.com/fullsend-ai/fullsend/internal/security" ) +var skillMarkerNames = [...]string{"SKILL.md", "skill.md", "Skill.md"} + // scanRuntimeContent runs InputPipeline on agent definition, SKILL.md files, and plugin JSON. func scanRuntimeContent(input runtime.BootstrapInput, failClosed bool) error { agentPath := input.AgentPath() @@ -72,7 +74,7 @@ func scanAgentFile(pipeline *security.Pipeline, agentPath string, failClosed boo func scanSkillDir(pipeline *security.Pipeline, skillPath string, failClosed bool) error { var skillContent []byte - for _, name := range []string{"SKILL.md", "skill.md", "Skill.md"} { + for _, name := range skillMarkerNames { if c, err := os.ReadFile(filepath.Join(skillPath, name)); err == nil { skillContent = c break diff --git a/internal/cli/skill_collision.go b/internal/cli/skill_collision.go index 5eafe83e30..6268ce7f9b 100644 --- a/internal/cli/skill_collision.go +++ b/internal/cli/skill_collision.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "io/fs" "os" "path/filepath" @@ -43,15 +44,21 @@ func warnRepoSkillCollisions(repoDir string, harnessSkillDirs []string, printer } 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 + return isReadableSkillMarkerFS(os.DirFS(skillDir)) +} + +func isReadableSkillMarkerFS(skillFS fs.FS) bool { + for _, marker := range skillMarkerNames { + info, err := fs.Stat(skillFS, marker) + if err != nil || !info.Mode().IsRegular() { + continue + } + file, err := skillFS.Open(marker) + if err != nil { + continue + } + _ = file.Close() + return true } - _ = file.Close() - return true + return false } diff --git a/internal/cli/skill_collision_test.go b/internal/cli/skill_collision_test.go index bbbcaf4d15..8b5c25a00f 100644 --- a/internal/cli/skill_collision_test.go +++ b/internal/cli/skill_collision_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "testing" + "testing/fstest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,6 +31,18 @@ func TestWarnRepoSkillCollisions_WarnsForShadowedSkill(t *testing.T) { assert.Contains(t, output.String(), "base: harness composition to override it") } +func TestIsReadableSkillMarkerFS_AcceptsSupportedCasing(t *testing.T) { + for _, marker := range []string{"SKILL.md", "skill.md", "Skill.md"} { + t.Run(marker, func(t *testing.T) { + skillFS := fstest.MapFS{ + marker: &fstest.MapFile{Data: []byte("# Skill")}, + } + + assert.True(t, isReadableSkillMarkerFS(skillFS)) + }) + } +} + func TestWarnRepoSkillCollisions_DoesNotWarnWithoutCollision(t *testing.T) { repoDir := t.TempDir() projectSkillsDir := filepath.Join(repoDir, ".claude", "skills")