diff --git a/docs/architecture/extensions/external-ai-work-sources-design.md b/docs/architecture/extensions/external-ai-work-sources-design.md index 943e8159f0..4e109e5448 100644 --- a/docs/architecture/extensions/external-ai-work-sources-design.md +++ b/docs/architecture/extensions/external-ai-work-sources-design.md @@ -382,10 +382,14 @@ Plugin Host Runtime、已退役的 LSP Runtime，以及通用动态模型路由 字符串或字符串列表；名称按参数顺序绑定并由现有纯文本参数展开器处理，缺失命名参数展开为空，既有缺失位置参数仍保留 占位符。`.codex` Skill 只增加上游已有的目录名 fallback，`description` 仍必填；`.agents`、`.opencode`、`.openbitfun` 和 `.cursor` 的严格格式不变。本地与 Remote 发现及实际加载必须使用同一方言映射，避免目录显示可用而执行时重新解析失败。 -- Claude Skill 的 `allowed-tools` 同样不能授予 OpenBitFun 工具预批准，因此安全降级为无额外权限；`effort` 只作为 reasoning profile 参与 - 现有显式模型绑定，不成为请求级 override；`context`/`fork`、`agent`、`model`、`hooks`、`paths`、`shell`、 - `runtime` 等会改变执行行为而当前没有等价 owner 的字段阻止加载。Claude runtime 变量与动态 - shell 注入也不执行。此切片不增加插件 Skill、祖先活动目录、文件 watcher、URL 来源或另一条 reload 命令。 +- Claude Skill 的 `allowed-tools` 不能授予 OpenBitFun 工具预批准，因此降级为无额外权限。`model`、`effort` 偏好不应用， + 继续使用当前会话配置，并在扫描诊断及实际加载结果中说明降级；它们不再阻止整份技能加载。 + Claude runtime 变量与动态 shell 表达式保留为未展开、未执行的文本，加载说明明确它们不是实际值或命令结果； + 如任务需要这些数据，Agent 必须通过当前工作区的正常工具与权限流程获取。行内 shell 表达式的识别要求行首或空白边界及闭合 + 反引号；普通 Markdown 中的感叹号、Excel `Sheet1!A1` 和错误值不会触发兼容性警告。 + `context`（包括 `fork`）、`agent`、`hooks`、`paths`、`shell`、`runtime`、`background`、`disallowed-tools` + 涉及未实现的执行方式或约束，仍阻止加载；格式损坏及无效调用控制字段也仍返回错误。本地与 Remote、名称与稳定键加载共享 + 同一兼容性判断和模型说明。此切片不增加插件 Skill、祖先活动目录、文件 watcher、URL 来源或另一条 reload 命令。 - Claude Subagent 扫描用户与逐层项目 `.claude/agents/**/*.md`，近工作目录定义整项覆盖；Claude MCP 保留 `local > project > user` 的整项覆盖，local 只读取与规范化当前工作区严格匹配的项目项。 - Codex Subagent 从用户与逐层项目 `[agents]`、角色文件合并，缺失字段按 Codex 层级继承；`enabled`、默认模型、角色级 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 763e0a35d1..8f1a7eaf6a 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 @@ -320,6 +320,7 @@ impl Tool for SkillTool { "description": skill_data.description, "location": location_str, "content": skill_data.content, + "compatibility_warnings": skill_data.compatibility_warnings, "success": true }), result_for_assistant: Some(result_for_assistant), @@ -444,7 +445,7 @@ Use the remote project skill. async fn read_file_text(&self, path: &str) -> anyhow::Result { if path == "/remote/project/.claude/skills/remote-review/SKILL.md" { return Ok( - "---\ndescription: Review a remote target.\narguments: target focus\n---\n\nReview $target for $focus.\n" + "---\ndescription: Review a remote target.\narguments: target focus\nmodel: opus\n---\n\nReview $target for $focus.\nContext: !`git diff`\n" .to_string(), ); } @@ -712,6 +713,23 @@ Use the remote project skill. #[tokio::test] async fn remote_claude_skill_uses_the_same_dialect_for_discovery_and_load() { let registry = SkillRegistry::global(); + let report = registry + .get_skill_scan_report_for_remote_workspace(&ClaudeRemoteFs, "/remote/project") + .await; + assert!(report + .skills + .iter() + .any(|skill| skill.name == "remote-review")); + assert_eq!( + report + .diagnostics + .iter() + .filter( + |notice| notice.path == "/remote/project/.claude/skills/remote-review/SKILL.md" + ) + .count(), + 2 + ); let visible = registry .get_resolved_skills_for_remote_workspace(&ClaudeRemoteFs, "/remote/project", None) .await; @@ -732,6 +750,8 @@ Use the remote project skill. assert_eq!(loaded.source_id, "claude-code"); assert_eq!(loaded.source_label, "Claude Code"); assert_eq!(loaded.argument_names, ["target", "focus"]); + assert_eq!(loaded.compatibility_warnings.len(), 2); + assert!(loaded.content.contains("!`git diff`")); let loaded_by_key = registry .find_and_load_skill_by_key_for_remote_workspace( @@ -744,6 +764,61 @@ Use the remote project skill. .expect("remote skill should retain source metadata when loaded by key"); assert_eq!(loaded_by_key.source_id, loaded.source_id); assert_eq!(loaded_by_key.source_label, loaded.source_label); + assert_eq!( + loaded_by_key.compatibility_warnings, + loaded.compatibility_warnings + ); + } + + #[tokio::test] + async fn local_claude_fallback_survives_discovery_and_explicit_tool_loading() { + let temp = tempfile::tempdir().expect("tempdir"); + let skill_dir = temp.path().join(".claude/skills/compatibility-review"); + fs::create_dir_all(&skill_dir).unwrap(); + let body = "Excel `!` and `#REF!`. Review $target. Context: !`git diff`"; + fs::write(skill_dir.join("SKILL.md"), format!( + "---\ndescription: Compatibility review.\nmodel: opus\neffort: high\narguments: target\n---\n{body}" + )).unwrap(); + let registry = SkillRegistry::global(); + let report = registry + .get_skill_scan_report_for_workspace(Some(temp.path())) + .await; + let skill = report + .skills + .iter() + .find(|skill| skill.name == "compatibility-review") + .unwrap(); + assert_eq!( + report + .diagnostics + .iter() + .filter(|notice| PathBuf::from(¬ice.path) == skill_dir.join("SKILL.md")) + .count(), + 3 + ); + let context = local_context(temp.path().to_path_buf()); + for command in [skill.name.as_str(), skill.key.as_str()] { + let results = SkillTool::new() + .call_impl( + &json!({"command": command, "arguments": "workbook.xlsx"}), + &context, + ) + .await + .unwrap(); + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &results[0] + else { + panic!("expected skill result"); + }; + assert_eq!(data["content"], body.replace("$target", "workbook.xlsx")); + assert_eq!(data["compatibility_warnings"].as_array().unwrap().len(), 3); + let rendered = result_for_assistant.as_deref().unwrap(); + assert!(rendered.contains("have not been executed")); + assert!(rendered.contains("current session configuration")); + } } #[tokio::test] 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 ccfc7f065b..9d1f9d9493 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 @@ -197,7 +197,7 @@ impl SkillCandidateScan { fn into_candidates(self) -> Vec { for diagnostic in &self.diagnostics { warn!( - "Skill discovery incomplete: path={}, source={}, error={}", + "Skill discovery notice: path={}, source={}, detail={}", diagnostic.path, diagnostic.source_id, diagnostic.message ); } 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 cdada5a023..d72a509883 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 @@ -149,6 +149,13 @@ impl SkillRegistry { entry.slot, ) { Ok(mut data) => { + for warning in &data.compatibility_warnings { + scan.diagnostics.push(diagnostic( + &skill_md, + entry.source_id, + warning, + )); + } if let Some(error) = Self::apply_remote_openai_policy(&mut data, fs, &path).await { @@ -428,6 +435,13 @@ impl SkillRegistry { entry.slot, ) { Ok(mut data) => { + for warning in &data.compatibility_warnings { + scan.diagnostics.push(diagnostic( + skill_md.to_string_lossy(), + entry.source_id, + warning, + )); + } let (cacheable, policy_error) = Self::apply_local_openai_policy(&mut data, &path).await; scan.cacheable &= cacheable; diff --git a/src/crates/execution/agent-runtime/src/skills/types.rs b/src/crates/execution/agent-runtime/src/skills/types.rs index 051a513df9..6e1f62c299 100644 --- a/src/crates/execution/agent-runtime/src/skills/types.rs +++ b/src/crates/execution/agent-runtime/src/skills/types.rs @@ -19,7 +19,7 @@ pub struct SkillScanDiagnostic { impl SkillScanDiagnostic { pub fn to_xml(&self) -> String { let text = format!( - "Skill discovery incomplete at {} ({}): {}", + "Skill discovery notice at {} ({}): {}", self.path, self.source_id, self.message ); let escaped = text @@ -163,6 +163,8 @@ pub struct SkillData { pub allow_user_invocation: bool, pub argument_hint: Option, pub argument_names: Vec, + /// Compatibility notices accompany both discovery and explicit loading. + pub compatibility_warnings: Vec, } fn default_allow_implicit_invocation() -> bool { @@ -343,15 +345,13 @@ fn claude_argument_names(metadata: &Value) -> Result, SkillParseErro Ok(names) } -fn reject_unsupported_claude_semantics( +fn claude_compatibility_warnings( metadata: &Value, body: &str, -) -> Result<(), SkillParseError> { +) -> Result, SkillParseError> { const UNSUPPORTED_FIELDS: &[&str] = &[ "context", "agent", - "model", - "effort", "hooks", "paths", "shell", @@ -368,21 +368,45 @@ fn reject_unsupported_claude_semantics( ))); } + let mut warnings = Vec::new(); + for field in ["model", "effort"] { + if metadata.get(field).is_some() { + warnings.push(format!( + "Claude preference '{field}' is not applied; the current session configuration is used." + )); + } + } const DYNAMIC_MARKERS: &[&str] = &[ "${CLAUDE_SESSION_ID}", "${CLAUDE_EFFORT}", "${CLAUDE_SKILL_DIR}", - "!`", + "${CLAUDE_PROJECT_DIR}", ]; - if let Some(marker) = DYNAMIC_MARKERS - .iter() - .find(|marker| body.contains(**marker)) - { - return Err(SkillParseError::InvalidFormat(format!( - "Claude dynamic expression '{marker}' is not supported" - ))); + for marker in DYNAMIC_MARKERS { + if body.contains(marker) { + warnings.push(format!( + "Claude variable '{marker}' is preserved as literal text and has not been expanded. Do not assume it identifies a valid value or path." + )); + } + } + if has_claude_shell_expression(body) { + warnings.push( + "Claude shell expressions are preserved as literal text and have not been executed. They are not command results. If the task needs this data, obtain it through normal tools and permission checks in the active workspace.".to_string(), + ); } - Ok(()) + Ok(warnings) +} + +fn has_claude_shell_expression(body: &str) -> bool { + // Inline commands require a whitespace/start boundary and a closing backtick. + // In particular, Markdown code such as `!` and `#REF!` is ordinary text. + static INLINE: std::sync::LazyLock = std::sync::LazyLock::new(|| { + Regex::new(r"(?m)(?:^|\s)!`[^`\r\n]+`").expect("Claude inline expression regex") + }); + static FENCED: std::sync::LazyLock = std::sync::LazyLock::new(|| { + Regex::new(r"(?m)^ {0,3}```![ \t]*\r?$").expect("Claude fenced expression regex") + }); + INLINE.is_match(body) || FENCED.is_match(body) } impl SkillData { @@ -487,11 +511,12 @@ impl SkillData { { return Err(SkillParseError::MissingField("description")); } - let argument_names = if dialect == SkillSourceDialect::ClaudeCode { - reject_unsupported_claude_semantics(&metadata, &body)?; - claude_argument_names(&metadata)? + let (argument_names, compatibility_warnings) = if dialect == SkillSourceDialect::ClaudeCode + { + let warnings = claude_compatibility_warnings(&metadata, &body)?; + (claude_argument_names(&metadata)?, warnings) } else { - Vec::new() + (Vec::new(), Vec::new()) }; let allow_implicit_invocation = @@ -517,6 +542,7 @@ impl SkillData { allow_user_invocation, argument_hint, argument_names, + compatibility_warnings, }) } @@ -552,8 +578,16 @@ pub fn render_loaded_skill_for_assistant( String::new() }; + let warnings = if skill_data.compatibility_warnings.is_empty() { + String::new() + } else { + format!( + "\n\nCompatibility notes:\n- {}", + skill_data.compatibility_warnings.join("\n- ") + ) + }; format!( - "Skill '{}' loaded successfully{}. Note: any paths mentioned in this skill are relative to {}, not the workspace.\n\n\n{}\n", - skill_data.name, loaded_from, skill_data.path, skill_data.content + "Skill '{}' loaded successfully{}. Note: any paths mentioned in this skill are relative to {}, not the workspace.{}\n\n\n{}\n", + skill_data.name, loaded_from, skill_data.path, warnings, skill_data.content ) } diff --git a/src/crates/execution/agent-runtime/tests/agent_definition_contracts/skill_contracts.rs b/src/crates/execution/agent-runtime/tests/agent_definition_contracts/skill_contracts.rs index 9478ce6335..35188ef65d 100644 --- a/src/crates/execution/agent-runtime/tests/agent_definition_contracts/skill_contracts.rs +++ b/src/crates/execution/agent-runtime/tests/agent_definition_contracts/skill_contracts.rs @@ -180,8 +180,6 @@ fn claude_skill_rejects_unavailable_runtime_semantics() { for field in [ "context: fork", "agent: Explore", - "model: opus", - "effort: high", "hooks: {}", "paths: src/**", "shell: bash", @@ -201,25 +199,105 @@ fn claude_skill_rejects_unavailable_runtime_semantics() { .expect_err("unsupported Claude behavior must fail closed"); assert!(matches!(error, SkillParseError::InvalidFormat(_))); } +} +#[test] +fn claude_dynamic_content_loads_unchanged_with_compatibility_notes() { for body in [ "Use ${CLAUDE_SESSION_ID}.", "Use ${CLAUDE_EFFORT}.", "Read ${CLAUDE_SKILL_DIR}/data.", "Run !`git status` before continuing.", + "!`git diff`", + "Read ${CLAUDE_PROJECT_DIR}/data.", + "```!\ngit status\n```", ] { - let markdown = format!("---\ndescription: Dynamic behavior.\n---\n\n{body}\n"); - assert!(SkillData::from_markdown_for_source_slot( - "/workspace/.claude/skills/dynamic".to_string(), - &markdown, - SkillLocation::Project, + let markdown = format!("---\ndescription: Dynamic behavior.\n---\n\n{body}"); + for slot in ["claude", "home.claude"] { + let skill = SkillData::from_markdown_for_source_slot( + "/workspace/.claude/skills/dynamic".to_string(), + &markdown, + SkillLocation::Project, + true, + slot, + ) + .expect("dynamic content should load without executing or expanding it"); + assert_eq!(skill.content, body); + assert_eq!(skill.compatibility_warnings.len(), 1); + for stable_key in [false, true] { + let rendered = render_loaded_skill_for_assistant(&skill, stable_key); + assert!(rendered.contains(&skill.compatibility_warnings[0])); + assert!(rendered.contains(&format!("\n{body}\n"))); + } + let discovery = SkillData::from_markdown_for_source_slot( + skill.path.clone(), + &markdown, + SkillLocation::Project, + false, + slot, + ) + .unwrap(); + assert!(discovery.content.is_empty()); + assert_eq!( + discovery.compatibility_warnings, + skill.compatibility_warnings + ); + } + } +} + +#[test] +fn claude_excel_punctuation_and_non_command_backticks_need_no_fallback() { + let body = "Cross-sheet `!` references: `Sheet1!A1`. Errors: `#REF!`, `#DIV/0!`, `#VALUE!`.\nKEY=!`cmd`\nUnclosed !`command\nHello!"; + for slot in ["claude", "home.claude"] { + let skill = SkillData::from_markdown_for_source_slot( + "/skills/officecli-xlsx".into(), + &format!("---\nname: officecli-xlsx\ndescription: Excel workflows.\n---\n{body}"), + SkillLocation::User, true, - "claude", + slot, ) - .is_err()); + .unwrap(); + assert_eq!(skill.content, body); + assert!(skill.compatibility_warnings.is_empty()); } } +#[test] +fn claude_preferences_degrade_without_bypassing_execution_constraints() { + let markdown = "---\ndescription: Review.\nmodel: opus\neffort: high\ndisable-model-invocation: true\nuser-invocable: false\n---\nReview."; + let skill = SkillData::from_markdown_for_source_slot( + "/skills/review".into(), + markdown, + SkillLocation::User, + true, + "home.claude", + ) + .unwrap(); + assert_eq!(skill.compatibility_warnings.len(), 2); + assert!(!skill.allow_implicit_invocation); + assert!(!skill.allow_user_invocation); + assert_eq!(skill.content, "Review."); + let restricted = markdown.replace("model: opus", "model: opus\ndisallowed-tools: Write"); + assert!(SkillData::from_markdown_for_source_slot( + "/skills/review".into(), + &restricted, + SkillLocation::User, + true, + "home.claude", + ) + .is_err()); + let generic = SkillData::from_markdown_for_source_slot( + "/skills/review".into(), + &markdown.replace("description:", "name: review\ndescription:"), + SkillLocation::User, + true, + "openbitfun", + ) + .unwrap(); + assert!(generic.compatibility_warnings.is_empty()); +} + #[test] fn claude_skill_validates_argument_names_without_a_generic_schema() { for arguments in [