fix(skill): scan only depth-1 SKILL.md, ignore nested files (#1304)

[chenhg5]

Summary

cc-connect's SkillRegistry.discoverSkillsInDir recursed into every
subdirectory looking for SKILL.md files. This caused nested SKILL.md
files inside a skill's own asset tree (e.g.
frontend-design/references/finance-report/SKILL.md) to be registered
as sibling skills, leaking them as phantom slash commands into platform
command menus. Issue #1304 reports 101 leaked commands from one skill
(frontend-design) alone.

This change switches the scan to depth-1 only, matching the Claude Code
CLI convention:

<skill_dir>/<skill-name>/SKILL.md        — registered
<skill_dir>/<skill-name>/references/...  — asset, NOT registered

Changes

• core/skill.go: discoverSkillsInDir now scans immediate subdirectories
  only. The visited recursion-safety map and shouldDescendIntoSkillPath
  helper are gone. sameFilePath/realPath (only used by the removed
  recursion guard) are also removed. isDiscoverableSubdir keeps the same
  dir/symlink semantics for the depth-1 entry check.
• core/skill_test.go: 4 existing tests rewrote to use depth-1 layouts,
  added TestSkillRegistryListAll_IgnoresNestedSkillFiles (exact [Bug] Skill discovery recursively registers nested SKILL.md files — internal reference templates pollute the slash-command list #1304
  scenario) and TestSkillRegistryListAll_IgnoresSubdirWithoutSkillFile.
• core/interfaces.go: SkillProvider docstring updated to describe the
  depth-1 rule.
• CHANGELOG.md: unreleased Fixed entry.

Test plan

• go test ./core/ — all 7 skill tests pass; full core suite passes.
• go test ./config/ ./daemon/ — pass.
• go vet ./core/... ./agent/... ./config/... ./cmd/... ./daemon/... ./platform/... — clean.
• gofmt -l core/skill.go core/skill_test.go — clean.
• golangci-lint v2.11.4 run --new-from-rev origin/main ./core/ — 0 issues.

The fix is API-compatible: only the internal discovery walker changes.
Skill, SkillRegistry, Engine.ListSkills, and the management API
are unchanged.

Fixes #1304

🤖 Generated with Claude Code

[chenhg5]

结论: Approve

总体判断: PR #1317 干净实现 #1304 修复,depth-1 规则与 Claude Code CLI 一致,移除 4 个不再需要的辅助函数,新增 2 个 test 精确覆盖 reporter 场景与缺 SKILL.md 子目录。手测 101 phantom SKILL.md 全部忽略,只注册 1 个 skill,行为与 reporter 期望一致。可合并。

Review 范围:

• 看了 core/skill.go, core/skill_test.go, core/interfaces.go, CHANGELOG.md 完整 diff (+150/-76, 4 文件)。
• 重点关注 depth-1 行为正确性、向后兼容、对 14 platform 的影响、CHANGELOG 与 docstring 同步。

✅ 做得好的地方:

• discoverSkillsInDir 简化为 (scanRoot, seen) 双参数,函数体降 86 → 76 行,逻辑从"递归 + 同名跳过"变成"depth-1 列举 + 读 SKILL.md",意图更清晰。
• isDiscoverableSubdir 保留了 symlink 兼容(原 shouldDescendIntoSkillPath 行为),只评估 subdir,不评估 root 自身。
• TestSkillRegistryListAll_IgnoresNestedSkillFiles 精确复现 #1304 场景:depth-1 SKILL.md 注册,3 个 nested SKILL.md(references/finance-report、references/html-ppt-knowledge-arch-blueprint、references/deep/deeper)全部 ignore,resolves 也返回 nil。
• TestSkillRegistryListAll_IgnoresSubdirWithoutSkillFile 覆盖边缘:子目录无 SKILL.md 时静默跳过(且不递归找),适合 templates/、examples/ 这类资产目录。
• 4 个旧测试(_RecursesIntoGroupedDirectories / _FollowsDirectorySymlinks / _DoesNotLoopOnDirectorySymlinks / _DedupesByLeafDirectoryName / _IgnoresRootSkillFile)都改成 depth-1 布局 + 调整断言,无破坏性测试删除。
• 移除的辅助函数(shouldDescendIntoSkillPath / sameFilePath / realPath)在 main 上 0 callers,无遗留引用。
• SkillProvider docstring 显式说明 depth-1 规则 + 引用 issue #1304,未来 contributor 不易重新引入递归。

🚨/🔴 必须处理: 未发现。

🟠 建议改进 (P2, 不阻塞):

1. 现有用户若有 nested SKILL.md 在用:CHANGELOG entry 已点出,建议 release notes 也明确标注(可能影响已部署的 skills 目录布局,但根据 #1304 的实际现象,nested SKILL.md 是 skill 作者误放而非用户依赖,影响面应该很小)。
2. TestSkillRegistryListAll_FollowsDirectorySymlinks 改用 depth-1 symlink:旧版 symlink 的是 automation/ 整个 group 目录,新版 symlink 的是单个 skill 目录。语义略有变化:旧版 symlink 一个 group 等于 symlink 多 skill,新版只能 symlink 单 skill。建议在 SkillProvider docstring 加一句 "depth-1 symlinks for individual skills are supported, group symlinks are not" 避免用户踩坑。
3. dedup test 改成 2 目录 + 同名 helper:旧版用 1 目录 2 个深嵌套同名 helper,新版用 2 目录各 1 个 depth-1 helper,语义更清晰(跨 root dedup),但是说明"dedup across all configured dirs"在注释里需要点出来 — 已在 test 注释中体现,OK。

🔵 可选优化 (P3):

• discoverSkillsInDir 的 docstring 可以加一个 "matching Claude Code CLI" 引文链接到 Claude Code docs,方便未来 contributor cross-check。
• Invalidate cache 在 ListAll 之后如果用户新增 SKILL.md 不会自动 re-scan,这是预期行为但未在 test 中验证。建议单独加一个 "invalidate after directory change" test(不在本 PR scope)。

❓ 需要确认: 无。

Testing / Risk:

• 已验证:
  • go build -tags no_web ./core/... OK(exit 0)
  • go vet -tags no_web ./core/... clean
  • gofmt -l core/skill.go core/skill_test.go core/interfaces.go 无输出
  • go test -count=1 -tags no_web -run "TestSkillRegistryListAll" ./core/ 7/7 PASS(覆盖 depth-1 / nested / 缺 SKILL.md / symlink / 自循环 symlink / dedup / 根 SKILL.md)
  • go test -count=1 -tags no_web ./core/ 45.347s 全绿
  • 手测 #1304 精确场景:构造 frontend-design/ + 101 nested SKILL.md(在 25 个 phantom 子目录下变体 variant-000..variant-100),ListAll 只返回 1 个 "frontend-design",101 个 phantom 全部 ignore ✓
  • CI 5/5 green(lint 2m24s / unit 4m2s / smoke 30s / regression 31s / performance 47s,run 27364485338)
• 兼容: API 表面未变(Skill / SkillRegistry / Engine.ListSkills / management API)。14 platform 通过 SkillRegistry 间接消费,本 PR 不动 platform/* 或 agent/*,无 regression 风险。
• SUPERSEDED check: 已在 main 的 core/skill.go grep "discoverSkillsInDir|ListAll" — main 仍是递归版,本 PR 仍 unique。
• mergeable: MERGEABLE, mergeStateStatus: CLEAN。

Next step:
OK to merge。建议: 合并后让 reporter 复测 Discord 的 command menu(应只剩 1 个 frontend-design,而非 101 个 phantom),并在 release notes 加一行 "Skill discovery is now depth-1 only; nested SKILL.md files are treated as skill assets and ignored (#1304)"。

[chenhg5]

结论: Comment (实质 APPROVE, ready-to-merge after CI green)

总体判断: PR clean & low-risk, 0 P0/P1/P2/P3 blockers, ready for owner merge. 修复 #1304 的 phantom slash command leak, 跟 Claude Code CLI 行为对齐。

Review 范围:

• 看了 core/skill.go (refactor + dedup) / core/interfaces.go (docstring 更新) / core/skill_test.go (5 updated + 2 new) / CHANGELOG.md (Unreleased/### Fixed)
• 重点关注 correctness (depth-1 只注册 <root>/<name>/SKILL.md)、symlink 处理、dedupe 跨 root 行为、backward-compat。

✅ 做得好的地方:

• matches Claude Code CLI convention: depth-1 约定跟 upstream 一致, 不引入 cc-connect 专属概念, 文档明确引用 issue #1304。
• 去掉 recursive helpers: 删除 realPath / sameFilePath / visited map / shouldDescendIntoSkillPath recursive descender, 函数 signature 从 4-param 缩到 2-param (scanRoot, seen), 复杂度显著下降。 0 new utility, 0 new state machine complexity。
• 7 个 test 覆盖完整: depth-1 happy path / nested SKILL.md ignored (frontend-design with 101 leaked commands 真实 case) / subdir without SKILL.md silently skipped / symlinks at depth-1 / no-loop symlink / dedup across roots / root SKILL.md ignored. 7/7 PASS locally 0.010s。
• CHANGELOG entry 完整: Unreleased/### Fixed 段, 描述清晰 (101 leaked commands 量化), 引用 #1304, 跟 v1.3.3 release section 格式一致。
• API surface 0 改动: SkillProvider interface signature 没动, 只是 interface docstring 更新。 所有现有 callers (agent/claudecode 等) 0 改动, 0 backward-compat 风险。

🚨/🔴 必须处理: 无

🟠 建议改进: 无 P2

🔵 P3 nit: 无

❓ 需要确认: 无

Testing / Risk:

• 已看到的验证 (本地 2026-06-16):
  • 7/7 TestSkillRegistryListAll_* PASS (0.010s)
  • Full go test ./core/ -count=1 47.463s PASS (0 regression)
  • go vet ./core/ 0 issue
  • gofmt -l core/skill.go core/skill_test.go core/interfaces.go 0 diff
• dev-claudecode rebase 干净: CHANGELOG Unreleased 段 + core/interfaces.go auto-merge OK (bb618b0)
• 未覆盖风险: real-world skill dirs with non-standard layouts (e.g. symlinks pointing to depth-2 skill trees) — 建议 owner merge 后用真实 frontend-design skill repo 跑 smoke test 验证 phantom commands 消失, 但 7/7 tests 已 cover 关键 invariant。

Next step:

• owner 可直接 merge. dev-claudecode rebase 干净 + CI 5/5 PASS + 0 blocker.
• 顺手 follow-up: release-codex 把这条 entry 跟其他 v1.3.5 release scope PR 一起收齐 (#1266 / #1338 / #1299 / #1345 / #1353 + 本 #1317)。