feat(#2661): warn when repo skills are shadowed

[shairevivo]

Summary

Warn before Claude runs when a repo-level skill is shadowed by a same-named harness skill installed at the higher-precedence personal level. The warning points users to unique names for extension or base: harness composition for intentional overrides.

Related Issue

Closes #2661

Changes

• detect collisions against real .claude/skills/<name>/SKILL.md entries
• emit an actionable warning without changing existing precedence
• cover collision and non-collision behavior with filesystem-backed tests
• update runtime, user, and augmentation-skill guidance for the current ADR 0064 customization model

Testing

• make lint passes (stage changes first, then run)
• Tests added/updated for new or modified logic
• go test -race ./internal/cli -run TestWarnRepoSkillCollisions -count=1
• Approximate patch coverage: warnRepoSkillCollisions 93.3%
• make e2e-test locally — blocked before test execution because the local GitHub identity cannot create lock repositories in the halfsend-* pool orgs; CI will run with the repository's cross-org e2e credentials

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• Commits are signed off (DCO) — human and human-directed agent sessions only
• I wrote this contribution myself and can explain all changes in it

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[qodo-code-review]

PR Summary by Qodo

Warn when Claude repo skills are shadowed by harness skills

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Warn before Claude starts when harness skills shadow same-named repository skills.
• Validate collisions only for discoverable repository skills containing regular SKILL.md files.
• Document unique-name extension and base: composition override paths.

Diagram

graph TD
  A["Agent startup"] --> B{"Claude runtime?"} -->|Yes| C["Harness skills"] --> D["Collision check"] --> F{"Same skill name?"} -->|Yes| G["Warning output"]
  E["Repo skills"] --> D
  F -->|No| H["Continue bootstrap"]
  B -->|No| H

Loading

High-Level Assessment

The host-side Claude preflight is the appropriate approach: it uses the existing harness skill list, inspects the repository before upload, preserves established precedence, and avoids imposing Claude-specific semantics on other runtimes. Moving detection into runtime bootstrap would add coupling without improving behavior.

Files changed (7) +105 / -8

Enhancement (2) +43 / -0

run.goRun skill collision checks before Claude bootstrap +3/-0

Run skill collision checks before Claude bootstrap

• Invokes repository skill collision detection for Claude runtimes after harness bootstrap inputs are assembled and before sandbox content is installed.

internal/cli/run.go

skill_collision.goDetect and warn about shadowed repository skills +40/-0

Detect and warn about shadowed repository skills

• Builds a set of harness skill basenames, scans '.claude/skills', and warns for matching entries with a regular 'SKILL.md'. Missing or unreadable skill directories and non-skill entries are ignored without affecting startup.

internal/cli/skill_collision.go

Tests (1) +50 / -0

skill_collision_test.goCover collision warning and non-collision behavior +50/-0

Cover collision warning and non-collision behavior

• Adds filesystem-backed tests verifying actionable output for a real shadowed skill and no output for unique or invalid repository skill entries.

internal/cli/skill_collision_test.go

Documentation (4) +12 / -8

runtime-implementation.mdDocument warned skill shadowing in runtime precedence +1/-1

Document warned skill shadowing in runtime precedence

• Updates the runtime layering diagram to state that higher-precedence Fullsend skills still win while repository shadowing now emits a warning.

docs/contributing/runtime-implementation.md

customizing-agents.mdClarify collision warnings for agent customizations +2/-1

Clarify collision warnings for agent customizations

• Notes that same-named repository and built-in skills are ignored with a warning, directing users to the detailed precedence guidance.

docs/guides/user/customizing-agents.md

customizing-with-skills.mdExplain warned collisions and supported override paths +5/-3

Explain warned collisions and supported override paths

• Replaces the silent-shadowing guidance with the new warning behavior. Recommends unique skill names for extension and derived harness 'base:' composition for intentional overrides.

docs/guides/user/customizing-with-skills.md

SKILL.mdUpdate augmentation guidance for warned skill collisions +4/-3

Update augmentation guidance for warned skill collisions

• Teaches augmentation authors that same-named built-ins trigger warnings and documents unique naming versus 'base:' composition as the supported extension and override strategies.

skills/author-fullsend-augmentations/SKILL.md

[github-actions]

Site preview

Preview: https://cd4031bd-site.fullsend-ai.workers.dev

Commit: 7bdcd8dc09ab3bd8a710330588e236bc01e2c505

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

1. Non-skills trigger shadow warning ✓ Resolved 🐞 Bug ≡ Correctness

Description

warnRepoSkillCollisions adds every harness source basename without checking that the source
contains a regular SKILL.md, so a local configured directory that Claude cannot discover as a
personal skill still causes a valid repo skill with that basename to be reported as shadowed. The
run then uploads the marker-less directory successfully, making the warning incorrect even though
the repo skill remains the only discoverable skill.

Code

internal/cli/skill_collision.go[R15-17]

+	for _, skillDir := range harnessSkillDirs {
+		if skillDir != "" {
+			harnessSkills[filepath.Base(skillDir)] = struct{}{}

Relevance

●●● Strong

Concrete false-positive correctness issue; recent CLI history accepts narrowly scoped behavioral
fixes and regression coverage.

PR-#6784
PR-#6261

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The user guide defines a skill as a directory containing required SKILL.md; harness validation
only constrains URL entries and overrides, while Claude bootstrap uploads every supplied directory
without checking that marker. The new map construction therefore labels a class of accepted local
directories as higher-precedence skills even when they are not discoverable.

docs/guides/user/customizing-with-skills.md[12-16]
internal/harness/harness.go[952-982]
internal/runtime/claude.go[66-77]
internal/cli/skill_collision.go[14-18]
internal/cli/skill_collision.go[27-38]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Only include harness sources that are actual discoverable skills; a directory without a regular `SKILL.md` must not cause a repo skill to be reported as shadowed.

## Issue Context
Local harness skill paths are not currently validated for a `SKILL.md`, and Claude bootstrap uploads any configured directory. Mirror the marker check already applied to repository entries, including suitable tests for a marker-less harness directory.

## Fix Focus Areas
- internal/cli/skill_collision.go[14-18]
- internal/cli/skill_collision_test.go[15-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. base: jargon lacks definition ✓ Resolved 📜 Skill insight ✧ Quality

Description

The guide introduces derived harness and base: composition without defining or linking those
terms at their first use. Readers cannot reliably interpret the newly recommended override path
without searching later sections.

Code

docs/guides/user/customizing-with-skills.md[92]

+or use a derived harness with `base:` composition for an intentional override.

Relevance

●●● Strong

Recent docs precedent explicitly accepted defining or linking jargon on first use.

PR-#6455

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 1062083 requires jargon to be defined or linked on first occurrence. The changed line
introduces derived harness and base: composition without either treatment; the explanation
appears only later under the override section.

docs/guides/user/customizing-with-skills.md[88-92]
Skill: writing-user-docs

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Define or link the `derived harness` and `base:` composition terminology where the override path is first introduced.

## Issue Context
PR Compliance ID 1062083 requires domain-specific jargon in guides to be defined inline or linked on first use. The guide explains this mechanism later, so a direct link to that section is sufficient.

## Fix Focus Areas
- docs/guides/user/customizing-with-skills.md[92-92]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. E2E tests not passed 📘 Rule violation ▣ Testability

Description

This PR changes internal/cli/, but the PR description explicitly states that make e2e-test was
blocked and did not run locally, with no successful latest-commit result provided. The critical CLI
change therefore lacks the required evidence of a passing end-to-end test run.

Code

internal/cli/run.go[R1646-1648]

+	if rt.Name() == "claude" {
+		warnRepoSkillCollisions(hostRepositoryDir, boot.SkillDirs(), printer)
+	}

Relevance

●● Moderate

E2E evidence is explicitly missing, but history lacks a close precedent requiring this exact test
gate.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 1062051 applies to changes under internal/cli/ and requires evidence that make e2e-test
succeeded. The cited changed lines alter CLI execution behavior, while the configured E2E workflow
runs make e2e-test; the supplied PR description records the local test as blocked rather than
passed.

Rule 1062051: Run end-to-end tests for critical internal modules before merge
internal/cli/run.go[1646-1648]
.github/workflows/e2e.yml[168-170]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Obtain a successful `make e2e-test` result for the latest PR commit before merge.

## Issue Context
Changes under `internal/cli/` trigger PR Compliance ID 1062051. The repository workflow contains the E2E command, but the PR description says the local run was blocked and provides no successful CI result.

## Fix Focus Areas
- internal/cli/run.go[1646-1648]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 62 rules

✅ Skills: writing-user-docs, writing-adrs

Review mode: ⚖️ Balanced: This is a runtime behavior change in the CLI that affects skill discovery warnings, with filesystem edge cases and integration-path implications; it is more than a trivial localized edit, but not dense enough to warrant redundant extended review.

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗