feat(skill-manager): 增加 Skill 管理器#74
Merged
zerx-lab merged 2 commits intoMay 12, 2026
Merged
Conversation
Co-Authored-By: Warp <agent@warp.dev>
📝 WalkthroughWalkthroughAdds a left-panel Skill Manager with inventory grouping and duplicate detection, a browsable panel UI with search/filter/preview, workspace keybinding/toolbelt integration, scope/parse/conversion adjustments, localization, and tests. ChangesSkill Manager Feature
Sequence DiagramsequenceDiagram
participant User
participant Panel as SkillManagerPanel
participant Inventory as SkillManager
participant Editor as CodeEditor
User->>Panel: enter query / select provider / click row
Panel->>Inventory: list_skill_inventory(ctx)
Inventory-->>Panel: inventory items
User->>Panel: click edit
Panel->>Editor: OpenSkillFile (CodeSource::Skill)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/ai/skills/skill_utils.rs (1)
21-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win文档与实现不一致，需同步修正注释。
Line 37 与 Line 43 仍在描述“
dir_path不参与 dedup / name 唯一”，但当前实现已按(name, dir_path)去重。建议立即修正文档，避免后续调用方误判行为。✏️ 建议修改
-/// `dir_path` is the directory that owns the skill (仅用于定位 skill,不参与 dedup)。 +/// `dir_path` is the directory that owns the skill (参与 dedup key，和 name 共同决定唯一性)。 @@ -/// name-dedup 后 name 已唯一,reference 排序键退化为不会被触发的 tiebreak,但保留 -/// 以便未来若放宽 dedup 仍是稳定排序。 +/// 当前按 (name, owning directory) 去重；因此同名 skill 可在不同目录同时存在。 +/// reference 仍作为稳定排序的次级键，以保证输出顺序可复现。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/ai/skills/skill_utils.rs` around lines 21 - 44, The doc comment is out of sync with the implementation: update the comment in app/src/ai/skills/skill_utils.rs to state that deduplication uses the tuple (name, dir_path) (i.e., skills are deduped per name within each owning directory) rather than claiming dir_path “does not participate” and that name is globally unique; also keep the note that the returned Vec is sorted by (name, reference) for stable prompt-cache behavior. Concretely, edit the paragraphs referencing `skill_paths`, the sentence that says `dir_path` 不参与 dedup / name 唯一, and any lines claiming global name-uniqueness so they reflect the actual behavior (dedup key = (name, dir_path), names may repeat across directories) while preserving the explanation about stable sorting.
🧹 Nitpick comments (1)
app/src/workspace/view.rs (1)
598-601: ⚡ Quick win新增 Skill Manager 快捷键常量后，建议同步纳入缓存列表
Line 598 新增了
LEFT_PANEL_SKILL_MANAGER_BINDING_NAME，但KEYBINDINGS_TO_CACHE仍是 4 项定义。若未包含新绑定，可能导致该快捷键的缓存/恢复行为不一致。建议确认并补齐。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/workspace/view.rs` around lines 598 - 601, The new constant LEFT_PANEL_SKILL_MANAGER_BINDING_NAME was added but not included in the KEYBINDINGS_TO_CACHE array; update the KEYBINDINGS_TO_CACHE definition to include LEFT_PANEL_SKILL_MANAGER_BINDING_NAME alongside existing entries (e.g., ASK_AI_ASSISTANT_KEYBINDING_NAME) so the skill manager binding is cached/restored consistently and adjust the array length if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/ai/skills/dummy_skill_manager.rs`:
- Line 55: The parameter naming in dummy implementation list_skill_inventory is
inconsistent: change the signature of pub fn list_skill_inventory(&self, _ctx:
&AppContext) -> Vec<SkillInventoryItem> to use the standardized name ctx (pub fn
list_skill_inventory(&self, ctx: &AppContext) -> Vec<SkillInventoryItem>) and
explicitly ignore it inside the body (e.g., let _ = ctx; or drop(ctx);) so it
matches real implementations; update any call sites that passed/expected the
parameter name accordingly.
In `@app/src/ai/skills/skill_manager.rs`:
- Line 373: 将 list_skill_inventory 的未使用上下文参数从 `_ctx: &AppContext` 重命名为 `ctx:
&AppContext`（保持其为最后一个参数），并在函数体开始处添加 `let _ = ctx;` 来显式忽略该值；同时在所有调用
list_skill_inventory 的位置更新签名/调用以使用 ctx 而不是省略或使用
`_ctx`，以符合仓库关于上下文参数命名和值处理的约定（参考函数名 list_skill_inventory 和类型 AppContext 以定位修改点）。
In `@app/src/ai/skills/telemetry.rs`:
- Around line 20-21: 摘要：在添加了 SkillManager 来源后，需要更新 Skill.Opened
事件描述以包含该来源，避免埋点消费者误读。请在 app/src/ai/skills/telemetry.rs 中找到 Skill.Opened
事件的描述文本并修改，以明确列出所有触发来源（当前的编辑按钮、跳转到 /edit-skill 的行为以及新加入的 SkillManager
面板入口），并说明不同来源是否携带不同参数或上下文（如 skill_id、source="SkillManager" 等），以保证事件语义与新增的
SkillManager 枚举值一致。
In `@app/src/skill_manager/panel.rs`:
- Around line 295-301: The meta string uses hardcoded English labels
("default"/"duplicate") which won't localize; update the code that builds meta
(variable meta, using duplicate.provider/duplicate.scope, and branches on
has_duplicates/is_default) to append translated labels instead of raw
English—call the project's i18n translation function (the same one used
elsewhere in the panel) with appropriate keys (e.g., "skill.default" and
"skill.duplicate") and concatenate those localized strings into meta so the
label follows the rest of the panel translations.
In `@app/src/workspace/view.rs`:
- Around line 5560-5567: 当前代码在调用 source.path().unwrap_or_default()
时会在路径为空时传入空字符串导致打开失败或错误；请改为先对 source.path() 做显式判空（例如 match/if let Some(path) =
source.path()），只有在 Some(path) 时才调用 self.open_file_with_target(...)，并保持使用
EditorSettings::as_ref(ctx).open_file_layout 和 FileTarget::CodeEditor(layout)
等现有符号；如果需要处理 None 分支，请实现明确的回退逻辑或错误日志而不是使用空路径兜底。
- Around line 18864-18865: The UI currently always pushes
ToolPanelView::SkillManager while the handler OpenSkillFile is compiled out
without the local_fs feature; change the unconditional push to the same feature
gating pattern used by GlobalSearch and WarpDrive by wrapping the push in a
conditional that ensures both the build-time local_fs feature and the runtime
FeatureFlag are checked (e.g., use cfg!(feature = "local_fs") combined with
FeatureFlag::SkillManager.is_enabled()), so the ToolPanelView::SkillManager
entry is only added when OpenSkillFile will actually be available.
---
Outside diff comments:
In `@app/src/ai/skills/skill_utils.rs`:
- Around line 21-44: The doc comment is out of sync with the implementation:
update the comment in app/src/ai/skills/skill_utils.rs to state that
deduplication uses the tuple (name, dir_path) (i.e., skills are deduped per name
within each owning directory) rather than claiming dir_path “does not
participate” and that name is globally unique; also keep the note that the
returned Vec is sorted by (name, reference) for stable prompt-cache behavior.
Concretely, edit the paragraphs referencing `skill_paths`, the sentence that
says `dir_path` 不参与 dedup / name 唯一, and any lines claiming global
name-uniqueness so they reflect the actual behavior (dedup key = (name,
dir_path), names may repeat across directories) while preserving the explanation
about stable sorting.
---
Nitpick comments:
In `@app/src/workspace/view.rs`:
- Around line 598-601: The new constant LEFT_PANEL_SKILL_MANAGER_BINDING_NAME
was added but not included in the KEYBINDINGS_TO_CACHE array; update the
KEYBINDINGS_TO_CACHE definition to include LEFT_PANEL_SKILL_MANAGER_BINDING_NAME
alongside existing entries (e.g., ASK_AI_ASSISTANT_KEYBINDING_NAME) so the skill
manager binding is cached/restored consistently and adjust the array length if
necessary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 50c79a2e-18ba-428f-9870-6bc6d5384312
📒 Files selected for processing (25)
app/i18n/en/warp.ftlapp/i18n/zh-CN/warp.ftlapp/src/ai/agent_sdk/driver.rsapp/src/ai/skills/dummy_skill_manager.rsapp/src/ai/skills/listed_skill.rsapp/src/ai/skills/mod.rsapp/src/ai/skills/skill_manager.rsapp/src/ai/skills/skill_manager_tests.rsapp/src/ai/skills/skill_utils.rsapp/src/ai/skills/skill_utils_tests.rsapp/src/ai/skills/telemetry.rsapp/src/app_state.rsapp/src/code/view.rsapp/src/lib.rsapp/src/skill_manager/mod.rsapp/src/skill_manager/panel.rsapp/src/view_components/markdown_toggle_view.rsapp/src/workspace/action.rsapp/src/workspace/action_tests.rsapp/src/workspace/mod.rsapp/src/workspace/view.rsapp/src/workspace/view/left_panel.rscrates/ai/src/skills/conversion.rscrates/ai/src/skills/parse_skill.rscrates/ai/src/skills/skill_provider.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/ai/skills/skill_utils.rs (1)
66-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick win补上“同长度 reference”时的稳定 tiebreak
Line 66-69 仅比较长度；当同 rank 且长度相等时会保留“先遍历到的项”。如果
skill_paths的来源顺序有波动，这里会导致胜出项不稳定，与注释中“稳定输出/缓存命中”的目标冲突。建议在长度相等时再按字面序比较。建议修复
Entry::Occupied(mut e) => { let new_rank = provider_rank(descriptor.provider); let existing_rank = provider_rank(e.get().provider); if new_rank < existing_rank { e.insert(descriptor); - } else if new_rank == existing_rank - && skill_reference_key(&descriptor.reference).len() - < skill_reference_key(&e.get().reference).len() - { - e.insert(descriptor); + } else if new_rank == existing_rank { + let new_ref = skill_reference_key(&descriptor.reference); + let existing_ref = skill_reference_key(&e.get().reference); + if new_ref.len() < existing_ref.len() + || (new_ref.len() == existing_ref.len() && new_ref < existing_ref) + { + e.insert(descriptor); + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/ai/skills/skill_utils.rs` around lines 66 - 69, When comparing candidates in the tie-breaking block (the branch using skill_reference_key(&descriptor.reference) vs skill_reference_key(&e.get().reference)) add a stable lexical fallback: if new_rank == existing_rank and the reference key lengths are equal, compare the two keys with a lexicographic/string compare and choose the smaller (or deterministic) one so selection is stable across runs; update the logic around skill_reference_key(&descriptor.reference) / e.get().reference to include this extra comparison so equal-length keys are resolved deterministically.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/workspace/view.rs`:
- Around line 20668-20672: The ToggleSkillManager branch unconditionally toggles
the SkillManager panel (in the match arm handling ToggleSkillManager using
left_panel_view.as_ref(ctx).active_view() == ToolPanelView::SkillManager and
calling toggle_left_panel_view(&LeftPanelAction::SkillManager,...)), but the
SkillManager entry is not registered when the local_fs feature is disabled; add
the same runtime feature gate used by the entry point: wrap the
ToggleSkillManager handling with a check like FeatureFlag::local_fs.is_enabled()
(or the project's corresponding FeatureFlag variant) and only perform the
active_view check and call toggle_left_panel_view when that flag is enabled,
ensuring behavior matches the entry registration at runtime.
---
Outside diff comments:
In `@app/src/ai/skills/skill_utils.rs`:
- Around line 66-69: When comparing candidates in the tie-breaking block (the
branch using skill_reference_key(&descriptor.reference) vs
skill_reference_key(&e.get().reference)) add a stable lexical fallback: if
new_rank == existing_rank and the reference key lengths are equal, compare the
two keys with a lexicographic/string compare and choose the smaller (or
deterministic) one so selection is stable across runs; update the logic around
skill_reference_key(&descriptor.reference) / e.get().reference to include this
extra comparison so equal-length keys are resolved deterministically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fd70ba00-9f83-4e52-b68d-a7e980acd47f
📒 Files selected for processing (8)
app/i18n/en/warp.ftlapp/i18n/zh-CN/warp.ftlapp/src/ai/skills/dummy_skill_manager.rsapp/src/ai/skills/skill_manager.rsapp/src/ai/skills/skill_utils.rsapp/src/ai/skills/telemetry.rsapp/src/skill_manager/panel.rsapp/src/workspace/view.rs
✅ Files skipped from review due to trivial changes (1)
- app/i18n/zh-CN/warp.ftl
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/ai/skills/telemetry.rs
- app/src/ai/skills/dummy_skill_manager.rs
- app/i18n/en/warp.ftl
- app/src/ai/skills/skill_manager.rs
- app/src/skill_manager/panel.rs
Comment on lines
+20668
to
+20672
| ToggleSkillManager => { | ||
| let is_showing = | ||
| self.left_panel_view.as_ref(ctx).active_view() == ToolPanelView::SkillManager; | ||
| self.toggle_left_panel_view(&LeftPanelAction::SkillManager, is_showing, ctx); | ||
| } |
There was a problem hiding this comment.
为 ToggleSkillManager 增加与入口一致的 feature 门控
Line 20668-20672 目前无条件执行切换；但 Line 18869 在 local_fs 关闭时不会注册该视图。这里仍可能通过快捷键/动作触发到不可用路径，建议加同样的 local_fs 门控（如有运行时 FeatureFlag，也应一并对齐）。
建议修改
ToggleSkillManager => {
- let is_showing =
- self.left_panel_view.as_ref(ctx).active_view() == ToolPanelView::SkillManager;
- self.toggle_left_panel_view(&LeftPanelAction::SkillManager, is_showing, ctx);
+ if cfg!(feature = "local_fs") {
+ let is_showing = self.left_panel_view.as_ref(ctx).active_view()
+ == ToolPanelView::SkillManager;
+ self.toggle_left_panel_view(&LeftPanelAction::SkillManager, is_showing, ctx);
+ }
}As per coding guidelines, "UI entry points and corresponding code paths must use the same Feature Flag" and "Prefer runtime FeatureFlag::Xxx.is_enabled() checks over #[cfg(...)] conditional compilation".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/workspace/view.rs` around lines 20668 - 20672, The ToggleSkillManager
branch unconditionally toggles the SkillManager panel (in the match arm handling
ToggleSkillManager using left_panel_view.as_ref(ctx).active_view() ==
ToolPanelView::SkillManager and calling
toggle_left_panel_view(&LeftPanelAction::SkillManager,...)), but the
SkillManager entry is not registered when the local_fs feature is disabled; add
the same runtime feature gate used by the entry point: wrap the
ToggleSkillManager handling with a check like FeatureFlag::local_fs.is_enabled()
(or the project's corresponding FeatureFlag variant) and only perform the
active_view check and call toggle_left_panel_view when that flag is enabled,
ensuring behavior matches the entry registration at runtime.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
增加 OpenWarp 的 Skill 管理器左侧面板入口，只合入
feature/skill-manager中 Skill 管理相关功能：SKILL.md内容。本 PR 不包含
feature/skill-manager分支里的 OpenWarp release、Homebrew、上游同步脚本或无关paths.rs平台路径改动。Testing
cargo test -p warp skill_manager --libcargo test -p warp skill_utils --libcargo test -p ai skills::skill_provider::tests::warp_home_skill_path_uses_warp_provider --libcargo check -p warp --libgit diff --check未运行完整
cargo fmt --check/./script/presubmit：当前基线存在与本 PR 无关的格式化差异，例如app/build.rs、app/src/ai/agent/api/convert_conversation.rs、app/src/ai/agent_sdk/driver/output.rs等会导致全仓 fmt check 失败。为保持 PR 干净，本次没有把这些无关格式化改动纳入提交。Server API dependencies
Agent Mode
Changelog Entries for Stable
CHANGELOG-IMPROVEMENT: Add an OpenWarp Skill Manager panel for browsing and opening local skills.
Co-Authored-By: Warp agent@warp.dev
Summary by CodeRabbit
New Features
Improvements