From 43a1355de8b588d77e65c1b79de09ff957e1ccf1 Mon Sep 17 00:00:00 2001 From: limityan Date: Thu, 30 Jul 2026 09:08:58 +0800 Subject: [PATCH] feat(skills): support explicit invocation arguments --- docs/architecture/cli-product-line-design.md | 19 ++- src/apps/cli/src/modes/chat/capabilities.rs | 11 +- src/apps/cli/src/ui/skill_selector.rs | 23 ++++ src/apps/cli/src/ui/startup.rs | 11 +- .../claude-code-adapter/src/command_source.rs | 53 +------- .../tests/command_source.rs | 20 +++ .../tools/implementations/skill_tool.rs | 127 +++++++++++++++++- .../tools/implementations/skills/registry.rs | 15 ++- .../tools/implementations/skills/resolver.rs | 4 + .../execution/agent-runtime/src/skills/mod.rs | 7 +- .../agent-runtime/src/skills/selection.rs | 11 ++ .../agent-runtime/src/skills/types.rs | 28 ++++ .../agent-runtime/tests/skill_contracts.rs | 87 +++++++++++- .../services/services-core/src/markdown.rs | 108 +++++++++++++++ .../tests/markdown_owner_contracts.rs | 67 ++++++++- .../src/flow_chat/components/ChatInput.tsx | 23 ++-- .../utils/skillPromptReference.test.ts | 15 +++ .../flow_chat/utils/skillPromptReference.ts | 7 + .../src/infrastructure/config/types/index.ts | 4 + 19 files changed, 560 insertions(+), 80 deletions(-) diff --git a/docs/architecture/cli-product-line-design.md b/docs/architecture/cli-product-line-design.md index 3c19503dee..acb9e95891 100644 --- a/docs/architecture/cli-product-line-design.md +++ b/docs/architecture/cli-product-line-design.md @@ -516,7 +516,8 @@ Hook C0：脱敏发现 -> 精确命令预览 | 指纹确认 -> 原子发布本 仍等于导入值的字段；用户后续修改、来源变化或部分重新导入造成冲突时，逐字段选择“保留 BitFun / 重新导入 外部 / 手工处理”，不得整批覆盖。 -下表描述目标覆盖范围；当前能力仅限上文列出的 MCP C0a 与 Hook C0，不能由本表推导出其他资产已经实现。 +下表描述目标覆盖范围；显式配置导入能力仍仅限上文列出的 MCP C0a 与 Hook C0。Skill 的原地发现与调用使用下文所述 +的既有 Skill Registry 路径，不属于显式配置导入，也不能由本表推导出其他资产已经实现。 | 来源 | 目标可导入 | 目标不导入 | |---|---|---| @@ -536,7 +537,21 @@ Hook C0：脱敏发现 -> 精确命令预览 | 指纹确认 -> 原子发布本 展示来源和默认覆盖状态，模式配置再展示实际采用项；固定根顺序保持为 Skill Registry 的独立回归契约。 Skill Registry 还保留来源资产声明的隐式调用意图：Claude `SKILL.md` 的 `disable-model-invocation: true` 与 Codex `agents/openai.yaml` 的 `policy.allow_implicit_invocation: false` 都会让 Skill 不进入模型自动目录，但不影响 `/skills`、 -模式配置和显式加载。BitFun 不继承来源产品的全局启停策略，也未实现 URL/额外根和自动变化监听。 +模式配置和显式加载。Claude `user-invocable: false` 与上述模型调用策略相互独立：它只让 Skill 不进入 Web/CLI 的用户 +调用选择器，不从管理目录删除，也不改变模型目录或现有启停状态。缺省时 Skill 可由用户调用；`argument-hint` 只作为 +选择器提示显示，不自动写入输入框。Web 与 CLI/TUI 选择 Skill 后统一生成 `[$skill-name]` 引用，用户可以直接在后面继续 +输入参数，不需要先导入、复制或学习第二种启用流程。 + +显式调用仍由现有 `SkillTool` 和 Skill Registry 加载实际优先级赢家，本地与 Remote 分支沿用同一加载语义。工具的可选 +`arguments` 字段使用共享的静态模板展开：支持原始 `$ARGUMENTS`、从零开始的 `$ARGUMENTS[N]` 和 `$N`、单/双引号 +分组以及 `\$` 转义；缺失的位置参数保留原占位符，模板没有未转义占位符时才追加 `ARGUMENTS:` 段。该展开器只处理 +字符串，不执行命令、脚本或动态变量。未携带 `arguments` 的旧工具调用保持原 Skill 正文不变。 + +这项能力不新增导入记录、来源图、后台 watcher 或第二套刷新生命周期。工作区查询继续按现有 Registry 路径扫描，用户 +缓存继续使用已有刷新入口，CLI 的 `/reload-skills` 仍是明确的手动刷新方式；运行期不承诺对所有来源做文件监听或热重载。 +本切片也不实现 `allowed-tools`、`context`、`fork`、`agent`、`model`、命名参数、动态 shell/runtime 变量、URL、祖先目录 +级联、插件 Runtime 或 OpenCode 复杂 Hook。后续只有在存在稳定消费方和独立安全边界时才扩展这些语义。 + Skill 说明和索引可按 L1 处理，脚本、URL 和外部依赖按 L2 确认；显式导入仍不得复制凭据值。MCP 启用状态按 OpenCode 来源解释，首次连接、策略限制和凭据缺失分别显示。 diff --git a/src/apps/cli/src/modes/chat/capabilities.rs b/src/apps/cli/src/modes/chat/capabilities.rs index 0e5d58dd08..a899c9b321 100644 --- a/src/apps/cli/src/modes/chat/capabilities.rs +++ b/src/apps/cli/src/modes/chat/capabilities.rs @@ -52,14 +52,17 @@ impl ChatMode { rt_handle.block_on(async { let registry = SkillRegistry::global(); registry - .get_resolved_skills_for_workspace(Some(workspace.as_path()), Some(&agent_type)) + .get_user_invocable_skills_for_workspace( + Some(workspace.as_path()), + Some(&agent_type), + ) .await }) }); if skills.is_empty() { chat_state.add_system_message(format!( - "No enabled skills found for agent mode '{}'. Add skills in .bitfun/skills/, .cursor/skills/, or ~/.cursor/skills/, or enable built-in skills for this mode.", + "No user-invocable skills found for agent mode '{}'. Add or enable a skill, then check its user-invocable metadata.", self.agent_type )); return; @@ -133,7 +136,7 @@ impl ChatMode { /// Apply skill selection: fill input box with execution command fn apply_skill_selection(&self, selected: &SkillItem, chat_view: &mut ChatView) { - chat_view.set_input(&format!("Execute the {} skill.", selected.name)); + chat_view.set_input(&selected.invocation_text()); } fn set_skill_enabled( @@ -211,6 +214,7 @@ impl ChatMode { default_enabled: true, is_shadowed: info.is_shadowed, shadowed_by_key: info.shadowed_by_key, + argument_hint: info.argument_hint, } } @@ -227,6 +231,7 @@ impl ChatMode { default_enabled: info.default_enabled, is_shadowed: info.skill.is_shadowed, shadowed_by_key: info.skill.shadowed_by_key, + argument_hint: info.skill.argument_hint, } } diff --git a/src/apps/cli/src/ui/skill_selector.rs b/src/apps/cli/src/ui/skill_selector.rs index 15e0da3409..098abd9bcb 100644 --- a/src/apps/cli/src/ui/skill_selector.rs +++ b/src/apps/cli/src/ui/skill_selector.rs @@ -31,9 +31,14 @@ pub(crate) struct SkillItem { pub default_enabled: bool, pub is_shadowed: bool, pub shadowed_by_key: Option, + pub argument_hint: Option, } impl SkillItem { + pub(crate) fn invocation_text(&self) -> String { + format!("[${}] ", self.name) + } + fn display_source_label(&self) -> &str { let label = self.source_label.trim(); if !label.is_empty() { @@ -476,6 +481,15 @@ impl SkillSelectorState { )); } spans.push(Span::styled(skill.name.clone(), name_style)); + if let Some(argument_hint) = skill + .argument_hint + .as_deref() + .map(str::trim) + .filter(|hint| !hint.is_empty()) + { + spans.push(Span::raw(" ")); + spans.push(Span::styled(argument_hint.to_string(), desc_style)); + } if !status.is_empty() { spans.push(Span::styled(status, theme.style(StyleKind::Muted))); } @@ -532,9 +546,18 @@ mod tests { default_enabled: true, is_shadowed: false, shadowed_by_key: None, + argument_hint: None, } } + #[test] + fn skill_invocation_text_uses_the_shared_inline_token_without_inserting_the_hint() { + let mut skill = skill_item("project::bitfun::pdf", "BitFun"); + skill.argument_hint = Some("[file] [focus]".to_string()); + + assert_eq!(skill.invocation_text(), "[$pdf] "); + } + #[test] fn skill_coverage_uses_winner_source_label() { let winner = skill_item("project::bitfun::pdf", "BitFun"); diff --git a/src/apps/cli/src/ui/startup.rs b/src/apps/cli/src/ui/startup.rs index 91aeb14000..8c27085aa2 100644 --- a/src/apps/cli/src/ui/startup.rs +++ b/src/apps/cli/src/ui/startup.rs @@ -1923,14 +1923,17 @@ impl StartupPage { tokio::runtime::Handle::current().block_on(async { let registry = SkillRegistry::global(); registry - .get_resolved_skills_for_workspace(Some(workspace.as_path()), Some(&agent_type)) + .get_user_invocable_skills_for_workspace( + Some(workspace.as_path()), + Some(&agent_type), + ) .await }) }); if skills.is_empty() { self.status = Some(format!( - "No enabled skills found for agent mode '{}'.", + "No user-invocable skills found for agent mode '{}'.", self.agent_type )); return; @@ -1978,7 +1981,7 @@ impl StartupPage { SkillSelectorAction::ConfigureSkills => self.show_skill_config_selector(), SkillSelectorAction::Execute(selected) => { self.skill_selector.hide(); - self.set_input(&format!("Execute the {} skill.", selected.name)); + self.set_input(&selected.invocation_text()); } SkillSelectorAction::Toggle(selected) => { self.set_skill_enabled(&selected, !selected.enabled); @@ -2053,6 +2056,7 @@ impl StartupPage { default_enabled: true, is_shadowed: info.is_shadowed, shadowed_by_key: info.shadowed_by_key, + argument_hint: info.argument_hint, } } @@ -2069,6 +2073,7 @@ impl StartupPage { default_enabled: info.default_enabled, is_shadowed: info.skill.is_shadowed, shadowed_by_key: info.skill.shadowed_by_key, + argument_hint: info.skill.argument_hint, } } diff --git a/src/crates/adapters/claude-code-adapter/src/command_source.rs b/src/crates/adapters/claude-code-adapter/src/command_source.rs index 24fac199af..73f6ec5de5 100644 --- a/src/crates/adapters/claude-code-adapter/src/command_source.rs +++ b/src/crates/adapters/claude-code-adapter/src/command_source.rs @@ -5,7 +5,7 @@ use bitfun_product_domains::external_sources::{ PromptCommandDefinition, PromptCommandProviderIdentity, PromptCommandProviderSnapshot, PromptCommandSourceProvider, SourceKey, SourceQualifiedCommandId, }; -use bitfun_services_core::markdown::FrontMatterMarkdown; +use bitfun_services_core::markdown::{expand_prompt_template_arguments, FrontMatterMarkdown}; use bitfun_static_hook_support::{ collect_bounded_regular_files, read_bounded_text, BoundedDirectoryWalkError, BoundedDirectoryWalkLimits, BoundedTextRead, @@ -230,7 +230,7 @@ impl PromptCommandSourceProvider for ClaudeCodeCommandProvider { } match &command.availability { PromptCommandAvailability::Available => Ok(ExpandedPromptCommand { - content: expand_template(&command.template, arguments), + content: expand_prompt_template_arguments(&command.template, arguments), }), PromptCommandAvailability::Restricted { reason, .. } | PromptCommandAvailability::Invalid { reason } => { @@ -686,55 +686,6 @@ fn command_definition( Ok(definition) } -fn expand_template(template: &str, arguments: &str) -> String { - let args = argument_regex() - .find_iter(arguments) - .map(|item| { - let value = item.as_str(); - if value.len() >= 2 - && ((value.starts_with('"') && value.ends_with('"')) - || (value.starts_with('\'') && value.ends_with('\''))) - { - value[1..value.len() - 1].to_string() - } else { - value.to_string() - } - }) - .collect::>(); - let with_positions = - placeholder_regex().replace_all(template, |capture: ®ex::Captures<'_>| { - let position = capture - .get(1) - .or_else(|| capture.get(2)) - .and_then(|value| value.as_str().parse::().ok()) - .unwrap_or(usize::MAX); - args.get(position).cloned().unwrap_or_default() - }); - let uses_arguments = template.contains("$ARGUMENTS"); - let uses_positions = placeholder_regex().is_match(template); - let mut expanded = with_positions.replace("$ARGUMENTS", arguments); - if !uses_arguments && !uses_positions && !arguments.trim().is_empty() { - expanded.push_str("\n\nARGUMENTS: "); - expanded.push_str(arguments); - } - expanded.trim().to_string() -} - -fn argument_regex() -> &'static Regex { - static REGEX: OnceLock = OnceLock::new(); - REGEX.get_or_init(|| { - Regex::new(r#"(?:\[Image\s+\d+\]|"[^"]*"|'[^']*'|[^\s"']+)"#) - .expect("static Claude Code argument regex compiles") - }) -} - -fn placeholder_regex() -> &'static Regex { - static REGEX: OnceLock = OnceLock::new(); - REGEX.get_or_init(|| { - Regex::new(r"\$(?:ARGUMENTS\[(\d+)\]|(\d+))").expect("static placeholder regex compiles") - }) -} - fn shell_regex() -> &'static Regex { static REGEX: OnceLock = OnceLock::new(); REGEX.get_or_init(|| Regex::new(r"!`[^`]+`").expect("static shell regex compiles")) diff --git a/src/crates/adapters/claude-code-adapter/tests/command_source.rs b/src/crates/adapters/claude-code-adapter/tests/command_source.rs index a203bf7f06..d850405620 100644 --- a/src/crates/adapters/claude-code-adapter/tests/command_source.rs +++ b/src/crates/adapters/claude-code-adapter/tests/command_source.rs @@ -241,6 +241,26 @@ fn arguments_without_a_placeholder_use_claude_codes_arguments_section() { ); } +#[test] +fn missing_and_escaped_argument_placeholders_remain_literal() { + let fixture = Fixture::new(); + write( + fixture.user_claude.join("commands/literal.md"), + r"Use $0, keep $ARGUMENTS[3], and show \$ARGUMENTS plus \$1", + ); + + let provider = fixture.provider(); + let snapshot = provider.discover(&fixture.context()).unwrap(); + + assert_eq!( + provider + .expand(&snapshot.commands[0], "alpha beta") + .unwrap() + .content, + "Use alpha, keep $ARGUMENTS[3], and show $ARGUMENTS plus $1" + ); +} + #[test] fn case_insensitive_duplicate_in_one_layer_is_invalid_and_deterministic() { let fixture = Fixture::new(); 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 6086cd39c3..b6a7e72282 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 @@ -8,6 +8,7 @@ use crate::agentic::tools::framework::{ }; use crate::util::errors::{BitFunError, BitFunResult}; use async_trait::async_trait; +use bitfun_services_core::markdown::expand_prompt_template_arguments; use log::debug; use serde_json::{json, Value}; @@ -29,15 +30,17 @@ impl SkillTool { When users ask you to perform tasks, check whether any skills listed in the current skill listing can help complete the task more effectively. Skills provide specialized capabilities and domain knowledge. How to use skills: -- Invoke skills using this tool with the listed skill name or stable key (no arguments) +- Invoke skills using this tool with the listed skill name or stable key and optional arguments +- Pass user-provided invocation text relevant to the skill through `arguments`; never copy an `argument-hint` into arguments - The skill's prompt will expand and provide detailed instructions on how to complete the task - Examples: - `command: "pdf"` - invoke the pdf skill + - `command: "review", arguments: "src/main.rs carefully"` - invoke a skill with arguments - `command: "xlsx"` - invoke the xlsx skill - `command: "user::bitfun-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 +- 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]` invocation - Do not invoke a skill that is already running "# .to_string() @@ -134,7 +137,11 @@ impl Tool for SkillTool { "properties": { "command": { "type": "string", - "description": "The skill name (no arguments). E.g., \"pdf\" or \"xlsx\"" + "description": "The skill name or stable key. E.g., \"pdf\" or \"user::bitfun-system::ppt-design\"" + }, + "arguments": { + "type": "string", + "description": "Optional arguments supplied to the skill prompt" } }, "required": ["command"], @@ -184,6 +191,17 @@ impl Tool for SkillTool { meta: None, }; } + if input + .get("arguments") + .is_some_and(|value| !value.is_string()) + { + return ValidationResult { + result: false, + message: Some("arguments must be a string".to_string()), + error_code: Some(400), + meta: None, + }; + } ValidationResult { result: true, @@ -216,7 +234,7 @@ impl Tool for SkillTool { // Find and load skill through registry let registry = get_skill_registry(); let use_stable_key = skill_name.split("::").count() == 3; - let skill_data = if context.is_remote() { + let mut skill_data = if context.is_remote() { if let Some(ws_fs) = context.ws_fs() { let root = context .workspace @@ -281,6 +299,9 @@ impl Tool for SkillTool { } }; + if let Some(arguments) = input.get("arguments").and_then(Value::as_str) { + skill_data.content = expand_prompt_template_arguments(&skill_data.content, arguments); + } let location_str = skill_data.location.as_str(); let result_for_assistant = render_loaded_skill_for_assistant(&skill_data, use_stable_key); @@ -403,6 +424,69 @@ Use the remote project skill. } } + fn local_context(root: PathBuf) -> crate::agentic::tools::framework::ToolUseContext { + crate::agentic::tools::framework::ToolUseContext { + tool_call_id: None, + agent_type: None, + session_id: None, + dialog_turn_id: None, + workspace: Some(WorkspaceBinding::new(None, root)), + loaded_deferred_tool_specs: Vec::new(), + primary_model_facts: tool_runtime::context::PrimaryModelFacts::default(), + custom_data: Default::default(), + computer_use_host: None, + runtime_tool_restrictions: Default::default(), + runtime_handles: bitfun_runtime_ports::ToolRuntimeHandles::new(None, None), + } + } + + #[test] + fn skill_schema_exposes_optional_arguments() { + let schema = SkillTool::new().input_schema(); + + assert_eq!(schema["properties"]["arguments"]["type"], "string"); + assert_eq!(schema["required"], json!(["command"])); + } + + #[tokio::test] + async fn skill_call_expands_arguments_in_loaded_prompt() { + let temp = tempfile::tempdir().expect("tempdir"); + let skill_dir = temp.path().join(".bitfun/skills/argument-skill"); + fs::create_dir_all(&skill_dir).expect("skill directory"); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: argument-skill\ndescription: Argument expansion test.\nargument-hint: \"[file] [focus]\"\n---\n\nReview $0 with $ARGUMENTS[1]. Full: $ARGUMENTS\n", + ) + .expect("skill markdown"); + let context = local_context(temp.path().to_path_buf()); + + let results = SkillTool::new() + .call_impl( + &json!({ + "command": "argument-skill", + "arguments": "\"src/main.rs\" carefully" + }), + &context, + ) + .await + .expect("skill arguments should expand"); + + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &results[0] + else { + panic!("expected result payload"); + }; + let expected = "Review src/main.rs with carefully. Full: \"src/main.rs\" carefully"; + assert_eq!(data["content"], expected); + assert!(result_for_assistant + .as_deref() + .unwrap_or_default() + .contains(expected)); + } + #[tokio::test] async fn remote_description_indexes_project_skills_through_workspace_services() { let identity = @@ -705,6 +789,9 @@ Use the remote project skill. let implicit = registry .get_implicitly_invocable_skills_for_workspace(Some(temp.path()), None) .await; + let user_invocable = registry + .get_user_invocable_skills_for_workspace(Some(temp.path()), None) + .await; assert!(resolved .iter() @@ -712,10 +799,42 @@ Use the remote project skill. assert!(!implicit .iter() .any(|skill| skill.name == "local-explicit-only")); + assert!(user_invocable + .iter() + .any(|skill| skill.name == "local-explicit-only")); let loaded = registry .find_and_load_skill_for_workspace("local-explicit-only", Some(temp.path()), None) .await .expect("explicit invocation should remain available"); assert_eq!(loaded.name, "local-explicit-only"); } + + #[tokio::test] + async fn local_user_invocation_metadata_hides_only_the_picker_catalog() { + let temp = tempfile::tempdir().expect("tempdir"); + let skill_dir = temp.path().join(".claude/skills/model-only"); + fs::create_dir_all(&skill_dir).expect("skill directory"); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: model-only\ndescription: model only\nuser-invocable: false\n---\n\nRun when useful.\n", + ) + .expect("skill markdown"); + + let registry = SkillRegistry::global(); + let resolved = registry + .get_resolved_skills_for_workspace(Some(temp.path()), None) + .await; + let implicit = registry + .get_implicitly_invocable_skills_for_workspace(Some(temp.path()), None) + .await; + let user_invocable = registry + .get_user_invocable_skills_for_workspace(Some(temp.path()), None) + .await; + + assert!(resolved.iter().any(|skill| skill.name == "model-only")); + assert!(implicit.iter().any(|skill| skill.name == "model-only")); + assert!(!user_invocable + .iter() + .any(|skill| skill.name == "model-only")); + } } 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 dafb4e48ba..9f44480360 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 @@ -13,8 +13,8 @@ use crate::infrastructure::get_path_manager_arc; use crate::util::errors::{BitFunError, BitFunResult}; use bitfun_agent_runtime::skills::{ annotate_shadowed_skills, build_mode_skill_infos, filter_candidates_for_mode, - filter_implicitly_invocable_skills, is_skill_globally_enabled, normalize_local_skill_dir_name, - normalize_remote_skill_dir_name, normalize_skill_keys, + filter_implicitly_invocable_skills, filter_user_invocable_skills, is_skill_globally_enabled, + normalize_local_skill_dir_name, normalize_remote_skill_dir_name, normalize_skill_keys, resolve_default_hidden_builtin_for_explicit_invocation, resolve_user_config_skill_root, resolve_visible_skills, sort_skill_candidates_by_dir, sort_skills, ExplicitSkillInvocationResolution, SkillCandidate, BITFUN_SKILL_SOURCE_ID, @@ -670,6 +670,17 @@ impl SkillRegistry { ) } + pub async fn get_user_invocable_skills_for_workspace( + &self, + workspace_root: Option<&Path>, + agent_type: Option<&str>, + ) -> Vec { + filter_user_invocable_skills( + self.get_resolved_skills_for_workspace(workspace_root, agent_type) + .await, + ) + } + pub async fn get_implicitly_invocable_skills_for_remote_workspace( &self, fs: &dyn WorkspaceFileSystem, diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/skills/resolver.rs b/src/crates/assembly/core/src/agentic/tools/implementations/skills/resolver.rs index 773c649184..8af83f3ef7 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/skills/resolver.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/skills/resolver.rs @@ -32,6 +32,8 @@ mod tests { is_shadowed: false, shadowed_by_key: None, allow_implicit_invocation: true, + allow_user_invocation: true, + argument_hint: None, } } @@ -51,6 +53,8 @@ mod tests { is_shadowed: false, shadowed_by_key: None, allow_implicit_invocation: true, + allow_user_invocation: true, + argument_hint: None, } } diff --git a/src/crates/execution/agent-runtime/src/skills/mod.rs b/src/crates/execution/agent-runtime/src/skills/mod.rs index 326af66a56..69119fb4f3 100644 --- a/src/crates/execution/agent-runtime/src/skills/mod.rs +++ b/src/crates/execution/agent-runtime/src/skills/mod.rs @@ -28,9 +28,10 @@ pub use roots::{ }; pub use selection::{ annotate_shadowed_skills, build_mode_skill_infos, filter_candidates_for_mode, - filter_implicitly_invocable_skills, is_skill_globally_enabled, normalize_skill_keys, - resolve_default_hidden_builtin_for_explicit_invocation, resolve_visible_skills, - sort_skill_candidates_by_dir, sort_skills, ExplicitSkillInvocationResolution, SkillCandidate, + filter_implicitly_invocable_skills, filter_user_invocable_skills, is_skill_globally_enabled, + normalize_skill_keys, resolve_default_hidden_builtin_for_explicit_invocation, + resolve_visible_skills, sort_skill_candidates_by_dir, sort_skills, + ExplicitSkillInvocationResolution, SkillCandidate, }; pub use types::{ render_loaded_skill_for_assistant, ModeSkillInfo, ModeSkillStateReason, SkillData, SkillInfo, diff --git a/src/crates/execution/agent-runtime/src/skills/selection.rs b/src/crates/execution/agent-runtime/src/skills/selection.rs index c73ae40562..754fb3f559 100644 --- a/src/crates/execution/agent-runtime/src/skills/selection.rs +++ b/src/crates/execution/agent-runtime/src/skills/selection.rs @@ -46,6 +46,8 @@ impl SkillCandidate { is_shadowed: false, shadowed_by_key: None, allow_implicit_invocation: data.allow_implicit_invocation, + allow_user_invocation: data.allow_user_invocation, + argument_hint: data.argument_hint, }, priority, } @@ -149,6 +151,13 @@ pub fn filter_implicitly_invocable_skills(skills: Vec) -> Vec) -> Vec { + skills + .into_iter() + .filter(|skill| skill.allow_user_invocation) + .collect() +} + pub fn is_skill_globally_enabled( skill: &SkillInfo, globally_disabled_user_skills: &HashSet, @@ -330,6 +339,8 @@ mod tests { is_shadowed: false, shadowed_by_key: None, allow_implicit_invocation: true, + allow_user_invocation: true, + argument_hint: None, }, priority: 0, } diff --git a/src/crates/execution/agent-runtime/src/skills/types.rs b/src/crates/execution/agent-runtime/src/skills/types.rs index 68549f3064..94c62c5d54 100644 --- a/src/crates/execution/agent-runtime/src/skills/types.rs +++ b/src/crates/execution/agent-runtime/src/skills/types.rs @@ -55,6 +55,10 @@ pub struct SkillInfo { pub shadowed_by_key: Option, #[serde(default = "default_allow_implicit_invocation", skip_serializing)] pub allow_implicit_invocation: bool, + #[serde(default = "default_allow_user_invocation")] + pub allow_user_invocation: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub argument_hint: Option, } impl SkillInfo { @@ -102,12 +106,18 @@ pub struct SkillData { pub source_slot: String, pub dir_name: String, pub allow_implicit_invocation: bool, + pub allow_user_invocation: bool, + pub argument_hint: Option, } fn default_allow_implicit_invocation() -> bool { true } +fn default_allow_user_invocation() -> bool { + true +} + fn optional_bool(metadata: &Value, field: &'static str) -> Result, SkillParseError> { let Some(value) = metadata.get(field) else { return Ok(None); @@ -141,6 +151,19 @@ fn optional_claude_bool( .ok_or_else(|| SkillParseError::InvalidFormat(format!("Field '{field}' must be a boolean"))) } +fn optional_string( + metadata: &Value, + field: &'static str, +) -> Result, SkillParseError> { + let Some(value) = metadata.get(field) else { + return Ok(None); + }; + value + .as_str() + .map(|value| Some(value.to_string())) + .ok_or_else(|| SkillParseError::InvalidFormat(format!("Field '{field}' must be a string"))) +} + fn parse_front_matter_markdown(content: &str) -> Result<(Value, String), SkillParseError> { static FRONT_MATTER_REGEX: std::sync::LazyLock = std::sync::LazyLock::new(|| { Regex::new(r"(?s)^---\r?\n(.*?)\r?\n---").expect("front matter regex pattern is valid") @@ -190,6 +213,9 @@ impl SkillData { let allow_implicit_invocation = !optional_claude_bool(&metadata, "disable-model-invocation")?.unwrap_or(false); + let allow_user_invocation = + optional_claude_bool(&metadata, "user-invocable")?.unwrap_or(true); + let argument_hint = optional_string(&metadata, "argument-hint")?; let skill_content = if with_content { body } else { String::new() }; let dir_name = Path::new(&path) @@ -208,6 +234,8 @@ impl SkillData { source_slot: String::new(), dir_name, allow_implicit_invocation, + allow_user_invocation, + argument_hint, }) } diff --git a/src/crates/execution/agent-runtime/tests/skill_contracts.rs b/src/crates/execution/agent-runtime/tests/skill_contracts.rs index 35128ae3a2..7f5e356474 100644 --- a/src/crates/execution/agent-runtime/tests/skill_contracts.rs +++ b/src/crates/execution/agent-runtime/tests/skill_contracts.rs @@ -3,8 +3,8 @@ use std::path::PathBuf; use bitfun_agent_runtime::skills::{ annotate_shadowed_skills, build_mode_skill_infos, builtin_skill_group_key, - filter_candidates_for_mode, filter_implicitly_invocable_skills, is_skill_globally_enabled, - render_loaded_skill_for_assistant, resolve_builtin_default_enabled, + filter_candidates_for_mode, filter_implicitly_invocable_skills, filter_user_invocable_skills, + is_skill_globally_enabled, render_loaded_skill_for_assistant, resolve_builtin_default_enabled, resolve_default_hidden_builtin_for_explicit_invocation, resolve_skill_default_enabled_for_mode, resolve_skill_state_for_mode, resolve_user_config_skill_root, resolve_visible_skills, sort_skills, ExplicitSkillInvocationResolution, ModeSkillStateReason, SkillCandidate, @@ -29,6 +29,8 @@ fn builtin_skill(dir_name: &str) -> SkillInfo { is_shadowed: false, shadowed_by_key: None, allow_implicit_invocation: true, + allow_user_invocation: true, + argument_hint: None, } } @@ -48,6 +50,8 @@ fn custom_user_skill(dir_name: &str) -> SkillInfo { is_shadowed: false, shadowed_by_key: None, allow_implicit_invocation: true, + allow_user_invocation: true, + argument_hint: None, } } @@ -67,6 +71,8 @@ fn project_skill(dir_name: &str) -> SkillInfo { is_shadowed: false, shadowed_by_key: None, allow_implicit_invocation: true, + allow_user_invocation: true, + argument_hint: None, } } @@ -179,11 +185,15 @@ fn skill_source_identity_is_serialized_without_changing_slot_identity() { let mut info = project_skill("pdf"); info.source_id = "bitfun".to_string(); info.source_label = "BitFun".to_string(); + info.allow_user_invocation = false; + info.argument_hint = Some("[file]".to_string()); let value = serde_json::to_value(info).expect("skill info should serialize"); assert_eq!(value["sourceSlot"], "bitfun"); assert_eq!(value["sourceId"], "bitfun"); assert_eq!(value["sourceLabel"], "BitFun"); + assert_eq!(value["allowUserInvocation"], false); + assert_eq!(value["argumentHint"], "[file]"); } #[test] @@ -323,6 +333,79 @@ Run the deployment workflow. assert!(!data.allow_implicit_invocation); } +#[test] +fn claude_user_invocation_metadata_is_independent_from_model_invocation() { + let markdown = r#"--- +name: deploy +description: Deploy the current project. +user-invocable: false +disable-model-invocation: false +argument-hint: "[environment] [version]" +--- + +Run the deployment workflow. +"#; + + let data = SkillData::from_markdown( + "/workspace/.claude/skills/deploy".to_string(), + markdown, + SkillLocation::Project, + false, + ) + .expect("valid Claude skill invocation metadata should parse"); + + assert!(!data.allow_user_invocation); + assert!(data.allow_implicit_invocation); + assert_eq!( + data.argument_hint.as_deref(), + Some("[environment] [version]") + ); +} + +#[test] +fn user_invocation_metadata_defaults_to_visible_without_an_argument_hint() { + let data = SkillData::from_markdown( + "/workspace/.agents/skills/review".to_string(), + "---\nname: review\ndescription: Review the current project.\n---\n\nReview it.\n", + SkillLocation::Project, + false, + ) + .expect("skill metadata defaults should parse"); + + assert!(data.allow_user_invocation); + assert_eq!(data.argument_hint, None); +} + +#[test] +fn invalid_user_invocation_metadata_is_rejected() { + for (field, value) in [("user-invocable", "[]"), ("argument-hint", "42")] { + let markdown = format!( + "---\nname: review\ndescription: Review the current project.\n{field}: {value}\n---\n\nReview it.\n" + ); + let error = SkillData::from_markdown( + "/workspace/.agents/skills/review".to_string(), + &markdown, + SkillLocation::Project, + false, + ) + .expect_err("invalid invocation metadata should fail closed"); + + assert!(error.to_string().contains(field), "field={field}"); + } +} + +#[test] +fn user_invocation_filter_keeps_only_picker_entries() { + let visible = project_skill("review"); + let mut model_only = project_skill("background-check"); + model_only.allow_user_invocation = false; + + let filtered = filter_user_invocable_skills(vec![model_only, visible]); + + assert_eq!(filtered.len(), 1); + assert_eq!(filtered[0].name, "review"); +} + #[test] fn claude_boolean_aliases_preserve_explicit_only_skill_visibility() { for value in ["yes", "ON", "1"] { diff --git a/src/crates/services/services-core/src/markdown.rs b/src/crates/services/services-core/src/markdown.rs index bcffeb0c38..09c361269a 100644 --- a/src/crates/services/services-core/src/markdown.rs +++ b/src/crates/services/services-core/src/markdown.rs @@ -6,6 +6,114 @@ static FRONT_MATTER_REGEX: LazyLock = LazyLock::new(|| { regex::Regex::new(r"(?s)^---\r?\n(.*?)\r?\n---").expect("front matter regex pattern is valid") }); +static PROMPT_ARGUMENT_REGEX: LazyLock = LazyLock::new(|| { + regex::Regex::new(r#"(?:\[Image\s+\d+\]|"[^"]*"|'[^']*'|[^\s"']+)"#) + .expect("prompt argument regex pattern is valid") +}); + +/// Expands Claude-compatible prompt arguments without executing dynamic content. +pub fn expand_prompt_template_arguments(template: &str, arguments: &str) -> String { + let arguments_by_position = PROMPT_ARGUMENT_REGEX + .find_iter(arguments) + .map(|item| { + let value = item.as_str(); + if value.len() >= 2 + && ((value.starts_with('"') && value.ends_with('"')) + || (value.starts_with('\'') && value.ends_with('\''))) + { + value[1..value.len() - 1].to_string() + } else { + value.to_string() + } + }) + .collect::>(); + + let mut expanded = String::with_capacity(template.len() + arguments.len()); + let mut cursor = 0; + let mut used_placeholder = false; + while cursor < template.len() { + let remaining = &template[cursor..]; + if remaining.starts_with(r"\\$") { + expanded.push_str(r"\\"); + cursor += 2; + continue; + } + if remaining.starts_with(r"\$") { + if let Some(length) = prompt_placeholder_length(&remaining[1..]) { + expanded.push_str(&remaining[1..length + 1]); + cursor += length + 1; + } else { + expanded.push('\\'); + cursor += 1; + } + continue; + } + if let Some((length, position)) = positional_placeholder(remaining) { + used_placeholder = true; + if let Some(argument) = position.and_then(|index| arguments_by_position.get(index)) { + expanded.push_str(argument); + } else { + expanded.push_str(&remaining[..length]); + } + cursor += length; + continue; + } + if let Some(length) = full_arguments_placeholder_length(remaining) { + used_placeholder = true; + expanded.push_str(arguments); + cursor += length; + continue; + } + + let character = remaining + .chars() + .next() + .expect("cursor is inside the template"); + expanded.push(character); + cursor += character.len_utf8(); + } + + if !used_placeholder && !arguments.trim().is_empty() { + expanded.push_str("\n\nARGUMENTS: "); + expanded.push_str(arguments); + } + expanded.trim().to_string() +} + +fn prompt_placeholder_length(value: &str) -> Option { + positional_placeholder(value) + .map(|(length, _)| length) + .or_else(|| full_arguments_placeholder_length(value)) +} + +fn full_arguments_placeholder_length(value: &str) -> Option { + let remaining = value.strip_prefix("$ARGUMENTS")?; + (!remaining.starts_with('[')).then_some("$ARGUMENTS".len()) +} + +fn positional_placeholder(value: &str) -> Option<(usize, Option)> { + if let Some(indexed) = value.strip_prefix("$ARGUMENTS[") { + let closing_bracket = indexed.find(']')?; + let index = &indexed[..closing_bracket]; + if !index.is_empty() && index.bytes().all(|byte| byte.is_ascii_digit()) { + return Some(( + "$ARGUMENTS[".len() + closing_bracket + 1, + index.parse::().ok(), + )); + } + } + + let indexed = value.strip_prefix('$')?; + let length = indexed + .bytes() + .take_while(|byte| byte.is_ascii_digit()) + .count(); + if length == 0 { + return None; + } + Some((length + 1, indexed[..length].parse::().ok())) +} + /// Parses and writes Markdown files with YAML front matter. pub struct FrontMatterMarkdown; diff --git a/src/crates/services/services-core/tests/markdown_owner_contracts.rs b/src/crates/services/services-core/tests/markdown_owner_contracts.rs index 8e2af7c283..a09358d58f 100644 --- a/src/crates/services/services-core/tests/markdown_owner_contracts.rs +++ b/src/crates/services/services-core/tests/markdown_owner_contracts.rs @@ -1,4 +1,4 @@ -use bitfun_services_core::markdown::FrontMatterMarkdown; +use bitfun_services_core::markdown::{expand_prompt_template_arguments, FrontMatterMarkdown}; use std::fs; #[test] @@ -19,3 +19,68 @@ fn front_matter_markdown_preserves_metadata_and_trimmed_body_contract() { assert!(saved.contains("tags:\n- one\n")); assert!(saved.ends_with("---\n\n# Saved\n")); } + +#[test] +fn prompt_arguments_expand_full_and_zero_based_quoted_positions() { + let expanded = expand_prompt_template_arguments( + "Full: $ARGUMENTS\nFirst: $0\nSecond: $ARGUMENTS[1]\nThird: $2", + "alpha \"two words\" 'three words'", + ); + + assert_eq!( + expanded, + "Full: alpha \"two words\" 'three words'\nFirst: alpha\nSecond: two words\nThird: three words" + ); +} + +#[test] +fn prompt_arguments_preserve_missing_and_escaped_placeholders() { + let expanded = expand_prompt_template_arguments( + r"Use $0, keep $ARGUMENTS[3], and show \$ARGUMENTS plus \$1", + "alpha beta", + ); + + assert_eq!( + expanded, + "Use alpha, keep $ARGUMENTS[3], and show $ARGUMENTS plus $1" + ); + + assert_eq!( + expand_prompt_template_arguments(r"Keep \\$0 expandable", "alpha"), + r"Keep \\alpha expandable" + ); + assert_eq!( + expand_prompt_template_arguments( + "Keep $ARGUMENTS[999999999999999999999999999999999999]", + "alpha", + ), + "Keep $ARGUMENTS[999999999999999999999999999999999999]" + ); +} + +#[test] +fn prompt_arguments_append_a_fallback_section_only_without_placeholders() { + assert_eq!( + expand_prompt_template_arguments("Review this change", "focus on auth"), + "Review this change\n\nARGUMENTS: focus on auth" + ); + assert_eq!( + expand_prompt_template_arguments(r"Show \$ARGUMENTS", "literally"), + "Show $ARGUMENTS\n\nARGUMENTS: literally" + ); + assert_eq!( + expand_prompt_template_arguments("Review this change", " "), + "Review this change" + ); +} + +#[test] +fn prompt_arguments_preserve_backslashes_before_non_placeholders() { + assert_eq!( + expand_prompt_template_arguments( + r"Keep \$HOME, \$ARGUMENTS[foo], and \${CLAUDE_SESSION_ID}", + "alpha", + ), + "Keep \\$HOME, \\$ARGUMENTS[foo], and \\${CLAUDE_SESSION_ID}\n\nARGUMENTS: alpha" + ); +} diff --git a/src/web-ui/src/flow_chat/components/ChatInput.tsx b/src/web-ui/src/flow_chat/components/ChatInput.tsx index f91f26d5be..05bf58e374 100644 --- a/src/web-ui/src/flow_chat/components/ChatInput.tsx +++ b/src/web-ui/src/flow_chat/components/ChatInput.tsx @@ -146,6 +146,7 @@ import { import { appendSkillPromptReferenceToken, createSkillPromptReferenceToken, + isSkillAvailableForUserInvocation, isSlashAddressableSkillName, replaceLeadingSlashCommandWithSkillToken, } from '../utils/skillPromptReference'; @@ -1036,9 +1037,9 @@ export const ChatInput: React.FC = ({ const setChatInputActive = useChatInputState(state => state.setActive); const setChatInputExpanded = useChatInputState(state => state.setExpanded); const setChatInputHeight = useChatInputState(state => state.setInputHeight); - const runtimeResolvedSkills = useMemo( - // Only surface skills that this mode will actually resolve at runtime. - () => resolvedModeSkills.filter(skill => skill.selectedForRuntime), + const userInvocableSkills = useMemo( + // Management keeps the full catalog; invocation surfaces apply both runtime and author visibility. + () => resolvedModeSkills.filter(isSkillAvailableForUserInvocation), [resolvedModeSkills] ); @@ -2674,7 +2675,7 @@ export const ChatInput: React.FC = ({ const q = (slashCommandState.query || '').trim().toLowerCase(); const seenNames = new Set(); - return runtimeResolvedSkills + return userInvocableSkills .filter(skill => { const normalizedName = skill.name.trim(); const normalizedNameKey = normalizedName.toLowerCase(); @@ -2698,7 +2699,9 @@ export const ChatInput: React.FC = ({ kind: 'skill' as const, id: skill.key, command: `/${skill.name}`, - label: skill.description || skill.name, + label: [skill.argumentHint?.trim(), skill.description || skill.name] + .filter(Boolean) + .join(' — '), skillName: skill.name, })) .sort((a, b) => { @@ -2708,7 +2711,7 @@ export const ChatInput: React.FC = ({ const bExact = bName === q ? 0 : bName.startsWith(q) ? 1 : 2; return aExact - bExact || aName.localeCompare(bName); }); - }, [canUseSkillsForTarget, runtimeResolvedSkills, slashCommandState.query]); + }, [canUseSkillsForTarget, slashCommandState.query, userInvocableSkills]); const resolveTypedMcpPromptCommand = useCallback((text: string): SlashMcpPromptItem | null => { const trimmed = text.trim(); @@ -5519,11 +5522,11 @@ export const ChatInput: React.FC = ({ {t('chatInput.boostSkillsLoading')} - ) : runtimeResolvedSkills.length === 0 ? ( + ) : userInvocableSkills.length === 0 ? (
{t('chatInput.boostSkillsEmpty')}
) : (
- {runtimeResolvedSkills.map(skill => ( + {userInvocableSkills.map(skill => (
= ({ onKeyDown={e => e.key === 'Enter' && insertSkillIntoInput(skill.name)} > - {skill.name} + + {[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 e2672080d5..6d5a40343a 100644 --- a/src/web-ui/src/flow_chat/utils/skillPromptReference.test.ts +++ b/src/web-ui/src/flow_chat/utils/skillPromptReference.test.ts @@ -3,6 +3,7 @@ import { appendSkillPromptReferenceToken, createSkillPromptReferenceToken, getSkillPromptReferenceMatches, + isSkillAvailableForUserInvocation, isSlashAddressableSkillName, parseSkillPromptReferenceToken, replaceLeadingSlashCommandWithSkillToken, @@ -48,4 +49,18 @@ describe('skillPromptReference', () => { expect(isSlashAddressableSkillName('browser-control')).toBe(true); expect(isSlashAddressableSkillName('browser control')).toBe(false); }); + + it('keeps user invocation visibility independent from runtime selection', () => { + expect(isSkillAvailableForUserInvocation({ + selectedForRuntime: true, + allowUserInvocation: true, + })).toBe(true); + expect(isSkillAvailableForUserInvocation({ + selectedForRuntime: true, + allowUserInvocation: false, + })).toBe(false); + expect(isSkillAvailableForUserInvocation({ + selectedForRuntime: false, + })).toBe(false); + }); }); diff --git a/src/web-ui/src/flow_chat/utils/skillPromptReference.ts b/src/web-ui/src/flow_chat/utils/skillPromptReference.ts index 814dee4a54..4ed27f1bd8 100644 --- a/src/web-ui/src/flow_chat/utils/skillPromptReference.ts +++ b/src/web-ui/src/flow_chat/utils/skillPromptReference.ts @@ -76,3 +76,10 @@ export function replaceLeadingSlashCommandWithSkillToken( export function isSlashAddressableSkillName(skillName: string): boolean { return SLASH_ADDRESSABLE_SKILL_NAME_PATTERN.test(skillName.trim()); } + +export function isSkillAvailableForUserInvocation(skill: { + selectedForRuntime: boolean; + allowUserInvocation?: boolean; +}): boolean { + return skill.selectedForRuntime && skill.allowUserInvocation !== false; +} diff --git a/src/web-ui/src/infrastructure/config/types/index.ts b/src/web-ui/src/infrastructure/config/types/index.ts index 66ed91072b..45a564b869 100644 --- a/src/web-ui/src/infrastructure/config/types/index.ts +++ b/src/web-ui/src/infrastructure/config/types/index.ts @@ -354,6 +354,10 @@ export interface SkillInfo { isShadowed?: boolean; /** Key of the skill that shadows this one (if any). */ shadowedByKey?: string | null; + /** False when the skill should stay out of user-facing invocation pickers. */ + allowUserInvocation?: boolean; + /** Optional usage hint displayed by invocation pickers. */ + argumentHint?: string | null; } export interface ModeSkillInfo extends SkillInfo {