From f2bd580e430d1958cf529ae9a8b90ce9cff31281 Mon Sep 17 00:00:00 2001 From: wsp Date: Wed, 9 Sep 2026 15:58:13 +0800 Subject: [PATCH] fix(skills): Align pickers and restore name tokens - Deduplicate all input skill lists using host-selected name winners. - Restore [$skill-name] references and remove source-key labels. - Remove exact-key prompt guidance and user-reference exceptions from the Skill tool instructions. - Preserve parsing of existing stable-key references. - Update reference tests and document the unified picker behavior. --- .../opencode-config-assets-adapter-design.md | 2 +- .../tools/implementations/skill_tool.rs | 3 +- .../tools/implementations/skills/registry.rs | 68 +++++++++---------- .../skills/registry/discovery.rs | 16 ++--- .../execution/agent-runtime/src/prompt.rs | 1 - .../src/flow_chat/components/ChatInput.tsx | 40 ++++++----- .../utils/skillPromptReference.test.ts | 5 +- .../flow_chat/utils/skillPromptReference.ts | 12 ++-- 8 files changed, 67 insertions(+), 80 deletions(-) diff --git a/docs/architecture/extensions/opencode-config-assets-adapter-design.md b/docs/architecture/extensions/opencode-config-assets-adapter-design.md index 25631f78d1..882ce85f38 100644 --- a/docs/architecture/extensions/opencode-config-assets-adapter-design.md +++ b/docs/architecture/extensions/opencode-config-assets-adapter-design.md @@ -246,7 +246,7 @@ watcher；用户通过统一的 `/reload instructions`（或默认 `/reload`） 标准 Skill 根使用有界递归发现（包括 Codex 的 `.system` 等容器目录），遇到 `SKILL.md` 后将该目录视为包边界，不继续收集其示例中的 Skill。 本地扫描跟随目录链接并按规范路径防环；远程扫描通过 workspace filesystem 跟随链接，以深度和目录预算终止循环。远程项目根与本地项目根采用相同的项目优先、用户次之顺序。 直接子目录保留原有 `scope::source-slot::directory` key；嵌套目录以根内 POSIX 相对路径作为末段，避免不同容器内同名目录冲突。 -按名称的默认选择维持现有覆盖规则；Web UI 的 `@` 技能选择器只显示当前模式按覆盖规则选出的名称赢家；显式选择以 `[$scope::source-slot::relative/path]` 调用精确 key，仍受全局、模式和作者的用户调用可见性约束。 +按名称的默认选择维持现有覆盖规则；Web UI 的 `@`、`/`、`$` 和输入框加号菜单中的技能列表只显示当前模式按覆盖规则选出的名称赢家；选择后插入 `[$技能名]`，仍受全局、模式和作者的用户调用可见性约束。已有的完整 key 引用继续兼容解析与调用。 扫描诊断与可用清单分别返回。Desktop 现有列表命令通过可选 `includeDiagnostics` 返回报告；参数缺省仍返回数组，新客户端接受旧主机的数组并标明诊断不可用。单个目录或文件失败不清空已发现技能。 diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skill_tool.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skill_tool.rs index 8f1a7eaf6a..ba44330ce4 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/skill_tool.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skill_tool.rs @@ -39,8 +39,7 @@ How to use skills: - `command: "user::openbitfun-system::ppt-design"` - invoke a specific built-in skill by stable key Important: -- Only use skills listed in the current skill listing's section, unless a trusted host task explicitly supplies an exact stable key or the user's message contains an exact `[$skill-name]` or `[$scope::source::directory]` invocation -- For an exact stable-key invocation, pass that key unchanged as `command`; never replace it with a same-named skill from another source +- Only use skills listed in the current skill listing's section, unless a trusted host task explicitly supplies an exact stable key - Do not invoke a skill that is already running "# .to_string() diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs index 9d1f9d9493..ca540f0912 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry.rs @@ -297,7 +297,7 @@ mod local_skill_scan_tests { let mut entry = test_root(&skills_path); entry.level = SkillLocation::Project; entry.slot = "agents"; - let scanned = SkillRegistry::scan_skills_in_dir(&entry).await; + let scanned = SkillRegistry::scan_skills_in_dir(&entry).await.candidates; assert_eq!(scanned.len(), 1); assert_eq!( scanned[0].info.installation_source.as_deref(), @@ -305,17 +305,19 @@ mod local_skill_scan_tests { ); entry.slot = "claude"; - assert!(SkillRegistry::scan_skills_in_dir(&entry).await[0] - .info - .installation_source - .is_none()); + assert!( + SkillRegistry::scan_skills_in_dir(&entry).await.candidates[0] + .info + .installation_source + .is_none() + ); entry.slot = "agents"; fs::write( temp.path().join("skills-lock.json"), "invalid existing user data", ) .unwrap(); - let scanned = SkillRegistry::scan_skills_in_dir(&entry).await; + let scanned = SkillRegistry::scan_skills_in_dir(&entry).await.candidates; assert_eq!(scanned.len(), 1); assert!(scanned[0].info.installation_source.is_none()); assert_eq!( @@ -375,7 +377,7 @@ mod local_skill_scan_tests { } let entry = test_root(root); - let scan = SkillRegistry::scan_skills_in_dir_with_status(&entry).await; + let scan = SkillRegistry::scan_skills_in_dir(&entry).await; assert!(!scan.cacheable); assert_eq!(scan.candidates.len(), 1); @@ -391,7 +393,7 @@ mod local_skill_scan_tests { return; } - let scan = SkillRegistry::scan_skills_in_dir_with_status(&test_root(root)).await; + let scan = SkillRegistry::scan_skills_in_dir(&test_root(root)).await; assert!(!scan.cacheable); assert!(scan.candidates.is_empty()); @@ -413,7 +415,7 @@ mod local_skill_scan_tests { return; } - let scan = SkillRegistry::scan_skills_in_dir_with_status(&test_root(root)).await; + let scan = SkillRegistry::scan_skills_in_dir(&test_root(root)).await; assert!(!scan.cacheable); assert_eq!(scan.candidates.len(), 1); @@ -436,7 +438,7 @@ mod local_skill_scan_tests { return; } - let scan = SkillRegistry::scan_skills_in_dir_with_status(&test_root(root)).await; + let scan = SkillRegistry::scan_skills_in_dir(&test_root(root)).await; assert!(!scan.cacheable); assert_eq!(scan.candidates.len(), 1); @@ -456,7 +458,7 @@ mod local_skill_scan_tests { .expect("skill markdown"); let entry = test_root(root); - let failed = SkillRegistry::scan_skills_in_dir_with_status(&entry).await; + let failed = SkillRegistry::scan_skills_in_dir(&entry).await; assert!(!failed.cacheable); assert!(failed.candidates[0].info.allow_implicit_invocation); @@ -469,7 +471,7 @@ mod local_skill_scan_tests { ) .expect("policy file"); - let recovered = SkillRegistry::scan_skills_in_dir_with_status(&entry).await; + let recovered = SkillRegistry::scan_skills_in_dir(&entry).await; assert!(recovered.cacheable); assert!(!recovered.candidates[0].info.allow_implicit_invocation); @@ -902,10 +904,6 @@ impl SkillRegistry { roots } - async fn scan_skills_in_dir(entry: &SkillRootEntry) -> Vec { - Self::scan_skills_in_dir_with_status(entry).await.candidates - } - async fn scan_user_skill_sources() -> UserSkillSources { #[cfg(feature = "file-watch")] let mut cacheable = match ensure_builtin_skills_installed().await { @@ -923,7 +921,7 @@ impl SkillRegistry { let mut standard = Vec::new(); let mut diagnostics = Vec::new(); for entry in Self::get_user_skill_roots() { - let mut scan = Self::scan_skills_in_dir_with_status(&entry).await; + let mut scan = Self::scan_skills_in_dir(&entry).await; #[cfg(feature = "file-watch")] { cacheable &= scan.cacheable; @@ -983,7 +981,7 @@ impl SkillRegistry { let mut standard = Vec::new(); if let Some(workspace_root) = workspace_root { for entry in Self::get_project_skill_roots(workspace_root) { - let mut part = Self::scan_skills_in_dir_with_status(&entry).await; + let mut part = Self::scan_skills_in_dir(&entry).await; standard.append(&mut part.candidates); diagnostics.append(&mut part.diagnostics); } @@ -1326,15 +1324,6 @@ impl SkillRegistry { } } - async fn scan_remote_project_skills( - fs: &dyn WorkspaceFileSystem, - remote_root: &str, - ) -> Vec { - Self::scan_remote_project_skills_with_diagnostics(fs, remote_root) - .await - .into_candidates() - } - async fn scan_skill_candidates_for_remote_workspace( &self, fs: &dyn WorkspaceFileSystem, @@ -1352,7 +1341,7 @@ impl SkillRegistry { ) -> SkillCandidateScan { let (user, project) = tokio::join!( self.scan_skill_candidates_with_diagnostics_for_workspace(None), - Self::scan_remote_project_skills_with_diagnostics(fs, remote_root), + Self::scan_remote_project_skills(fs, remote_root), ); Self::merge_remote_skill_scans(user, project) } @@ -2114,7 +2103,8 @@ mod opencode_configured_skill_tests { .unwrap(), is_builtin: false, }) - .await; + .await + .candidates; let configured = SkillRegistry::scan_configured_opencode_candidates(roots).await; let candidates = SkillRegistry::merge_configured_opencode_candidates(standard, configured, true); @@ -2142,7 +2132,8 @@ mod opencode_configured_skill_tests { priority: 0, is_builtin: false, }) - .await; + .await + .candidates; let configured = SkillRegistry::scan_configured_opencode_candidates(vec![configured_root( project.join("custom"), ExternalSourceScope::Project, @@ -2178,7 +2169,8 @@ mod opencode_configured_skill_tests { priority: super::PROJECT_SKILL_ROOTS.len(), is_builtin: false, }) - .await; + .await + .candidates; let configured = SkillRegistry::scan_configured_opencode_candidates(vec![ configured_root(home.join("configured"), ExternalSourceScope::UserGlobal, 0), configured_root(project.join("configured"), ExternalSourceScope::Project, 1), @@ -2438,7 +2430,9 @@ mod remote_scan_tests { ), ..Default::default() }; - let skills = SkillRegistry::scan_remote_project_skills(&fs, "/remote/project/").await; + let skills = SkillRegistry::scan_remote_project_skills(&fs, "/remote/project/") + .await + .into_candidates(); assert_eq!(skills.len(), 39); let installed = skills .iter() @@ -2453,7 +2447,9 @@ mod remote_scan_tests { ); fs.installation_lock = Some("invalid remote lock".into()); - let skills = SkillRegistry::scan_remote_project_skills(&fs, "/remote/project/").await; + let skills = SkillRegistry::scan_remote_project_skills(&fs, "/remote/project/") + .await + .into_candidates(); assert_eq!(skills.len(), 39); assert!(skills .iter() @@ -2464,7 +2460,9 @@ mod remote_scan_tests { async fn remote_scan_preserves_order_and_policy_with_bounded_io() { let fs = DelayedFs::default(); let start = Instant::now(); - let skills = SkillRegistry::scan_remote_project_skills(&fs, "/remote/project/").await; + let skills = SkillRegistry::scan_remote_project_skills(&fs, "/remote/project/") + .await + .into_candidates(); eprintln!( "remote scan: {:?}, {} logical IO calls, peak {}", start.elapsed(), @@ -2517,7 +2515,7 @@ mod remote_scan_tests { let start = Instant::now(); let mut local = Vec::new(); for entry in SkillRegistry::get_project_skill_roots(local_root.path()) { - local.extend(SkillRegistry::scan_skills_in_dir(&entry).await); + local.extend(SkillRegistry::scan_skills_in_dir(&entry).await.candidates); } eprintln!( "local project scan: {:?}, {} skills", diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry/discovery.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry/discovery.rs index d72a509883..f589b9fd47 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry/discovery.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skills/registry/discovery.rs @@ -53,7 +53,7 @@ fn flat_skill_data( } impl SkillRegistry { - pub(super) async fn scan_remote_project_skills_with_diagnostics( + pub(super) async fn scan_remote_project_skills( fs: &dyn WorkspaceFileSystem, remote_root: &str, ) -> SkillCandidateScan { @@ -322,7 +322,7 @@ impl SkillRegistry { scan } - pub(super) async fn scan_skills_in_dir_with_status(entry: &SkillRootEntry) -> LocalSkillScan { + pub(super) async fn scan_skills_in_dir(entry: &SkillRootEntry) -> LocalSkillScan { let mut scan = LocalSkillScan { candidates: Vec::new(), diagnostics: Vec::new(), @@ -685,7 +685,7 @@ mod tests { priority: 0, is_builtin: false, }; - let scan = SkillRegistry::scan_skills_in_dir_with_status(&entry).await; + let scan = SkillRegistry::scan_skills_in_dir(&entry).await; assert!(scan.diagnostics.is_empty()); assert_eq!(scan.candidates.len(), if slot == "pi" { 3 } else { 2 }); let flat = scan @@ -750,9 +750,7 @@ mod tests { #[tokio::test] async fn remote_flat_skill_discovery_and_loading_use_remote_posix_paths() { - let scan = - SkillRegistry::scan_remote_project_skills_with_diagnostics(&FlatRemote, "/remote") - .await; + let scan = SkillRegistry::scan_remote_project_skills(&FlatRemote, "/remote").await; assert!(scan.diagnostics.is_empty()); assert_eq!(scan.candidates.len(), 2); let pi = scan @@ -801,7 +799,7 @@ mod tests { priority: 0, is_builtin: false, }; - let scan = SkillRegistry::scan_skills_in_dir_with_status(&entry).await; + let scan = SkillRegistry::scan_skills_in_dir(&entry).await; let keys: HashSet<_> = scan .candidates .iter() @@ -875,9 +873,7 @@ mod tests { #[tokio::test] async fn remote_nested_links_errors_cycles_and_project_priority() { - let scan = - SkillRegistry::scan_remote_project_skills_with_diagnostics(&RemoteFixture, "/remote") - .await; + let scan = SkillRegistry::scan_remote_project_skills(&RemoteFixture, "/remote").await; assert_eq!(scan.candidates.len(), 2); assert!(scan .candidates diff --git a/src/crates/execution/agent-runtime/src/prompt.rs b/src/crates/execution/agent-runtime/src/prompt.rs index 02714bb8ab..8a04ba09f6 100644 --- a/src/crates/execution/agent-runtime/src/prompt.rs +++ b/src/crates/execution/agent-runtime/src/prompt.rs @@ -6,7 +6,6 @@ use serde::{Deserialize, Serialize}; const SKILL_LISTING_TITLE: &str = "# Skill Listing"; const SKILL_LISTING_GUIDANCE: &str = r#"A skill is a set of instructions provided through a `SKILL.md` source. If the user names a skill (with `[$SkillName]` or plain text) OR the task clearly matches a skill's description shown below, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned. -An explicit `[$scope::source::directory]` reference selects an exact installed skill. Pass the complete stable key to the Skill tool unchanged, even when another source has the same skill name. If that key is missing or disabled, report the error instead of substituting a same-named skill. Below is the list of skills that can be used with the Skill tool. Each entry includes a name and description"#; const AGENT_LISTING_TITLE: &str = "# Agent Listing"; const AGENT_LISTING_GUIDANCE: &str = "Available subagent types for the Task tool:"; diff --git a/src/web-ui/src/flow_chat/components/ChatInput.tsx b/src/web-ui/src/flow_chat/components/ChatInput.tsx index e37ff4f934..9b6bf20153 100644 --- a/src/web-ui/src/flow_chat/components/ChatInput.tsx +++ b/src/web-ui/src/flow_chat/components/ChatInput.tsx @@ -1511,21 +1511,19 @@ export const ChatInput: React.FC = ({ [resolvedModeSkills], ); const userInvocableSkills = useMemo( - // Management keeps the full catalog; invocation surfaces apply both runtime and author visibility. - () => resolvedModeSkills.filter(isSkillAvailableForUserInvocation), + // All input pickers use the host-selected winner for each skill name. + () => { + const seenNames = new Set(); + return resolvedModeSkills.filter(skill => { + if (!skill.selectedForRuntime || !isSkillAvailableForUserInvocation(skill) + || !skill.name.trim() || seenNames.has(skill.name)) return false; + seenNames.add(skill.name); + return true; + }); + }, [resolvedModeSkills] ); - const duplicateSkillNames = useMemo(() => { - const seen = new Set(); - const duplicates = new Set(); - for (const skill of userInvocableSkills) { - if (seen.has(skill.name)) duplicates.add(skill.name); - seen.add(skill.name); - } - return duplicates; - }, [userInvocableSkills]); - const quickSkillShortcuts = useMemo( () => canUseSkillsForTarget ? resolveChatInputQuickSkillShortcuts(resolvedModeSkills) @@ -3233,7 +3231,7 @@ export const ChatInput: React.FC = ({ kind: 'skill' as const, id: skill.key, command: `/${skill.name}`, - label: [duplicateSkillNames.has(skill.name) ? skill.key : undefined, skill.argumentHint?.trim(), skill.description || skill.name] + label: [skill.argumentHint?.trim(), skill.description || skill.name] .filter(Boolean) .join(' — '), skillName: skill.name, @@ -3245,7 +3243,7 @@ export const ChatInput: React.FC = ({ const bExact = bName === q ? 0 : bName.startsWith(q) ? 1 : 2; return aExact - bExact || aName.localeCompare(bName); }); - }, [canUseSkillsForTarget, duplicateSkillNames, slashCommandState.query, userInvocableSkills]); + }, [canUseSkillsForTarget, slashCommandState.query, userInvocableSkills]); const resolveTypedMcpPromptCommand = useCallback((text: string): SlashMcpPromptItem | null => { const trimmed = text.trim(); @@ -5226,14 +5224,14 @@ export const ChatInput: React.FC = ({ const replaceInlineTrigger = getRichTextTriggerController()?.replaceActiveInlineTrigger; if (inlineTriggerState.isActive) { - replaceInlineTrigger?.(createSkillPromptReferenceToken(item.skillName, item.id)); + replaceInlineTrigger?.(createSkillPromptReferenceToken(item.skillName)); setQueuedInput(null); setSlashCommandState({ isActive: false, kind: 'all', query: '', selectedIndex: 0 }); window.setTimeout(() => richTextInputRef.current?.focus(), 0); return; } - const next = replaceLeadingSlashCommandWithSkillToken(inputState.value, item.skillName, item.id); + const next = replaceLeadingSlashCommandWithSkillToken(inputState.value, item.skillName); dispatchInput({ type: 'SET_VALUE', payload: next }); inputValueRef.current = next; setQueuedInput(null); @@ -5593,13 +5591,13 @@ export const ChatInput: React.FC = ({ [dispatchInput, focusRichTextInputSoon, getRichTextTriggerController, inputState.value] ); - const insertSkillIntoInput = useCallback((skillName: string, skillKey?: string) => { - insertInlineReferenceIntoInput(createSkillPromptReferenceToken(skillName, skillKey)); + const insertSkillIntoInput = useCallback((skillName: string) => { + insertInlineReferenceIntoInput(createSkillPromptReferenceToken(skillName)); }, [insertInlineReferenceIntoInput]); const selectContextSkill = useCallback((skill: ContextPickerSkill) => { getRichTextTriggerController()?.replaceActiveContextTrigger?.( - createSkillPromptReferenceToken(skill.name, skill.key), + createSkillPromptReferenceToken(skill.name), ); setQueuedInput(null); focusRichTextInputSoon(); @@ -6502,10 +6500,10 @@ export const ChatInput: React.FC = ({ leading={} onClick={event => { event.stopPropagation(); - insertSkillIntoInput(skill.name, skill.key); + insertSkillIntoInput(skill.name); }} > - {[skill.name, duplicateSkillNames.has(skill.name) ? `(${skill.key})` : undefined, skill.argumentHint?.trim()].filter(Boolean).join(' ')} + {[skill.name, skill.argumentHint?.trim()].filter(Boolean).join(' ')} )) )} diff --git a/src/web-ui/src/flow_chat/utils/skillPromptReference.test.ts b/src/web-ui/src/flow_chat/utils/skillPromptReference.test.ts index a6107a6b28..51c6d29721 100644 --- a/src/web-ui/src/flow_chat/utils/skillPromptReference.test.ts +++ b/src/web-ui/src/flow_chat/utils/skillPromptReference.test.ts @@ -10,12 +10,11 @@ import { } from './skillPromptReference'; describe('skillPromptReference', () => { - it('preserves exact nested source identity alongside legacy name tokens', () => { + it('continues parsing existing exact nested source references', () => { const key = 'project::codex::.system/pdf'; - const token = createSkillPromptReferenceToken('pdf', key); + const token = `[$${key}]`; expect(token).toBe('[$project::codex::.system/pdf]'); expect(parseSkillPromptReferenceToken(token)).toEqual({ skillName: 'pdf', skillKey: key }); - expect(replaceLeadingSlashCommandWithSkillToken('/pdf read', 'pdf', key)).toBe(`${token} read`); expect(getSkillPromptReferenceMatches(`Use [$pdf] and ${token}`)).toHaveLength(2); }); diff --git a/src/web-ui/src/flow_chat/utils/skillPromptReference.ts b/src/web-ui/src/flow_chat/utils/skillPromptReference.ts index 9cf92085b6..e370c35aba 100644 --- a/src/web-ui/src/flow_chat/utils/skillPromptReference.ts +++ b/src/web-ui/src/flow_chat/utils/skillPromptReference.ts @@ -7,8 +7,8 @@ export interface SkillPromptReferenceTokenPayload { skillKey?: string; } -export function createSkillPromptReferenceToken(skillName: string, skillKey?: string): string { - return `[$${skillKey?.trim() || skillName.trim()}]`; +export function createSkillPromptReferenceToken(skillName: string): string { + return `[$${skillName.trim()}]`; } export function parseSkillPromptReferenceToken( @@ -59,9 +59,8 @@ export function getSkillPromptReferenceMatches(text: string): Array<{ export function appendSkillPromptReferenceToken( text: string, skillName: string, - skillKey?: string, ): string { - const token = createSkillPromptReferenceToken(skillName, skillKey); + const token = createSkillPromptReferenceToken(skillName); const trimmed = text.trimEnd(); return trimmed ? `${trimmed} ${token}` : token; } @@ -69,11 +68,10 @@ export function appendSkillPromptReferenceToken( export function replaceLeadingSlashCommandWithSkillToken( text: string, skillName: string, - skillKey?: string, ): string { - const token = createSkillPromptReferenceToken(skillName, skillKey); + const token = createSkillPromptReferenceToken(skillName); if (!text.trimStart().startsWith('/')) { - return appendSkillPromptReferenceToken(text, skillName, skillKey); + return appendSkillPromptReferenceToken(text, skillName); } return text.replace(LEADING_SLASH_COMMAND_PATTERN, (_match, whitespace: string) => `${whitespace}${token}`);