fix: support nested Claude skill directories#1697
fix: support nested Claude skill directories#1697Alexlangl wants to merge 6 commits intofarion1231:mainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
farion1231
left a comment
There was a problem hiding this comment.
感谢您的贡献,请查看一下有没有以下问题。:
-
[P1] Claude 同步目标路径只取叶子目录名,存在重名冲突。
app_relative_skill_path()对 Claude 只保留file_name(),这会让superpowers/brainstorming和tools/brainstorming都映射到同一个~/.claude/skills/brainstorming。这样在同步、删除和 cleanup 时都会互相覆盖/误删,indexed_skills里也只会保留其中一个。建议至少在导入或启用 Claude 时显式检测 leaf name 冲突,并阻止同名 skill 同时启用。 -
[P1] 扁平化映射后,旧的嵌套 live 目录不会被清理。
例如用户原本有~/.claude/skills/superpowers/brainstorming/SKILL.md,导入后同步回 Claude 会创建新的~/.claude/skills/brainstorming,但旧的superpowers/brainstorming不会被sync_to_app()清掉,因为当前 cleanup 只扫顶层目录。这样问题不只是“残留目录不好看”,而是旧 skill 仍可能继续被 Claude 读取,后续禁用/卸载时 CC Switch 也删不到它。 -
[P2] 已管理的嵌套 Claude skill 可能再次被扫成“未管理”。
数据库/SSOT 里保存的是完整相对路径,比如superpowers/brainstorming,但同步到 Claude live dir 后变成了叶子名brainstorming。这样下次scan_unmanaged()时,可能又把同一个 skill 当成新的 unmanaged skill 报出来,导致重复导入。 -
[P3] 小问题:注释里有个 typo。
list_app_skill_entries()的注释里写成了skillsg,应为skills。
建议补两类测试后再合:
- 同 leaf name、不同嵌套路径的 Claude skill 冲突场景。
- 嵌套 Claude skill 导入并同步后,再次
scan_unmanaged()不应重复出现。
|
感谢 review
另外按建议已经补充了两类测试
另外我手动在隔离环境里手动进行了更接近真实链路的验证:
|
farion1231
left a comment
There was a problem hiding this comment.
感谢您的更新,请查看一下是否存在以下问题:
[P2] Claude 的叶子目录冲突现在被“所有已安装 skill”占用,而不是“对 Claude 启用的 skill”占用。见 skill.rs (line 886) 和 skill.rs (line 1269)。collect_managed_app_paths(managed_skills.values()) 会把 superpowers/brainstorming 这样的 Codex-only skill 也映射成 Claude 的 brainstorming,结果是这个真实 Claude skill 会在未管理扫描里被跳过,并在 sync_to_app(&Claude) 时被 should_remove 删除。前面的 import_from_apps() 冲突检查只看 apps.claude,所以这里的行为明显更宽,容易误删用户已有的 Claude skill。
[P2] 嵌套 skill 的身份键还没全链路改成“完整 directory”。发现页在 SkillsPage.tsx (line 100) 仍用叶子名算 installed key,而 discoverable key 在 skill.rs (line 1681) 已经是完整路径;同样的叶子名 key 也还出现在 useSkills.ts (line 82) 和 UnifiedSkillsPanel.tsx (line 113)。这会让 superpowers/using-superpowers 这类 skill 在发现页继续显示未安装,安装/卸载后的 React Query 缓存更新也打不到正确条目,必须等全量刷新才恢复。
9138389 to
ff04f5d
Compare
回归测试
|
44d979b to
3d3c6c9
Compare
|
Closing this for now. I’ve already put a fair amount of time into it, and I don’t want to keep investing more without knowing whether it’s something you still want to move forward. If you’d like to revisit it later, feel free to let me know. |
|
Same message as in #1824 — these changes are welcome and I'd love to get them merged. Happy to help rebase if needed. |
efef847 to
9705c97
Compare
|
Closing this stale PR in favor of a fresh PR rebased onto current main. Continued in #2045. |
概要
主要解决Claude skill子目录问题
有些Claude skill拥有子目录,典型的就是
superpowersCC Switch 之前只能识别顶层目录,导致这类Claude skills 无法被正常发现、导入和统一管理。
变更
后端:
superpowers/brainstorming也就是说,现在这类结构:
可以被识别并导入,在同步到Cluade时,会按叶子skill处理,例如
superpowers/brainstorming会同步为~/.claude/skills/brainstorming。前端:
额外说明
此处改动只对 Claude 做了特判,没有扩大到其他 app。
主要是原因是这个 issue 对应的是 Claude skills 的目录组织方式和其他 app 有点不同:
验证
已通过
Related
Fix #1686
其它
是否需要不只是针对claude做单独处理?此处处理暂时没有将问题放大 @farion1231