Skip to content

Commit e40d7e8

Browse files
authored
Merge pull request #3008 from kev1n77/fmy/compatibility
fix(ecosystem): align source discovery and harden native imports
2 parents 531c557 + a02eae4 commit e40d7e8

29 files changed

Lines changed: 1140 additions & 137 deletions

File tree

‎docs/interactive-capabilities/technical/tauri-command-map.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@
329329
"visibility": "documented",
330330
"rustPath": "add_skill",
331331
"sourceFile": "src/apps/desktop/src/api/skill_api.rs",
332-
"signature": "fn add_skill( _state: State<'_, AppState>, source_path: String, level: String, workspace_path: Option<String>, source_key: Option<String>, target_name: Option<String>, ) -> Result<String, String>",
332+
"signature": "fn add_skill( _state: State<'_, AppState>, source_path: String, level: String, workspace_path: Option<String>, source_key: Option<String>, target_name: Option<String>, expected_source_fingerprint: Option<String>, ) -> Result<String, String>",
333333
"remoteWorkspacePolicy": "LegacyUnaudited"
334334
},
335335
{
@@ -10200,7 +10200,7 @@
1020010200
"visibility": "documented",
1020110201
"rustPath": "validate_skill_path",
1020210202
"sourceFile": "src/apps/desktop/src/api/skill_api.rs",
10203-
"signature": "fn validate_skill_path(path: String) -> Result<SkillValidationResult, String>",
10203+
"signature": "fn validate_skill_path( path: String, source_key: Option<String>, workspace_path: Option<String>, ) -> Result<SkillValidationResult, String>",
1020410204
"remoteWorkspacePolicy": "LegacyUnaudited"
1020510205
},
1020610206
{

‎scripts/core-boundaries/rules/source/public-api-rules.mjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,20 @@ export const opencodeAdapterPublicApiEntries = [
311311
'OpenCodeConfiguredSkillRoot',
312312
'openbitfun-core external source composition root and OpenCode configured Skill fixtures',
313313
),
314+
...['OpenCodeSkillRootDiagnostic', 'OpenCodeSkillRootReport'].map((symbol) => ({
315+
symbol,
316+
owner: 'opencode-adapter configured Skill root source owner',
317+
consumer: 'openbitfun-core external_sources::opencode_configured_skill_roots_with_provider',
318+
verification:
319+
'opencode_static_source_contracts::opencode_skill_roots diagnostics fixtures and core-boundary public API checks',
320+
p0: 'runtime-free configured local Skill root discovery with bounded diagnostics',
321+
contractSlice: contractSlices.opencodeAdapterBoundary,
322+
wireImpact: false,
323+
rationale:
324+
'Product Assembly projects valid roots and redacted discovery failures into its neutral Skill scan report without exposing OpenCode configuration syntax',
325+
exit:
326+
'remove only with the configured Skill root provider or an equivalent reviewed diagnostic projection',
327+
})),
314328
opencodeAdapterEntry(
315329
'OpenCodeSkillRootProvider',
316330
'openbitfun-core external source composition root and OpenCode configured Skill fixtures',

‎scripts/core-boundaries/self-test.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1638,7 +1638,7 @@ export function runManifestParserSelfTest({
16381638
).map((entry) => entry.symbol);
16391639
if (
16401640
opencodeAdapterPublicApiSymbols.join(',') !==
1641-
'load_opencode_package_adapter,load_opencode_config_snapshot,OpenCodeConfigSnapshot,OpenCodeConfigSnapshotError,project_plugin_config,project_plugin_tool_ref,OpenCodePluginConfigProjectionError,OpenCodeCommandProvider,OpenCodeCommandProviderOptions,OpenCodeConfiguredSkillRoot,OpenCodeSkillRootProvider,OpenCodeSkillRootProviderOptions,OpenCodeToolProvider,OpenCodeToolProviderOptions,OpenCodeSubagentProvider,OpenCodeSubagentProviderOptions,OpenCodeMcpProvider,OpenCodeMcpProviderOptions,OpenCodeHookProvider,OpenCodeHookProviderOptions,OpenCodeWorkspaceReferenceProvider,OpenCodeWorkspaceReferenceProviderOptions,load_opencode_user_instructions,OpenCodeInstructionSourceOptions'
1641+
'load_opencode_package_adapter,load_opencode_config_snapshot,OpenCodeConfigSnapshot,OpenCodeConfigSnapshotError,project_plugin_config,project_plugin_tool_ref,OpenCodePluginConfigProjectionError,OpenCodeCommandProvider,OpenCodeCommandProviderOptions,OpenCodeConfiguredSkillRoot,OpenCodeSkillRootDiagnostic,OpenCodeSkillRootReport,OpenCodeSkillRootProvider,OpenCodeSkillRootProviderOptions,OpenCodeToolProvider,OpenCodeToolProviderOptions,OpenCodeSubagentProvider,OpenCodeSubagentProviderOptions,OpenCodeMcpProvider,OpenCodeMcpProviderOptions,OpenCodeHookProvider,OpenCodeHookProviderOptions,OpenCodeWorkspaceReferenceProvider,OpenCodeWorkspaceReferenceProviderOptions,load_opencode_user_instructions,OpenCodeInstructionSourceOptions'
16421642
) {
16431643
throw new Error(
16441644
'OpenCode adapter public API budget must stay limited to the reviewed package factory, managed Plugin Host Config projection, and capability-specific command, configured Skill root, tool, subagent, MCP, static Hook, workspace Reference, and user Instruction providers',

‎src/apps/desktop/src/api/skill_api.rs‎

Lines changed: 102 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ use openbitfun_core::agentic::tools::implementations::skills::mode_overrides::{
2222
save_project_mode_skills_document_local, set_disabled_mode_skills_in_document,
2323
set_global_user_skill_disabled, set_mode_skill_disabled_in_document, set_user_mode_skill_state,
2424
};
25+
use openbitfun_core::agentic::tools::implementations::skills::registry::imports::{
26+
self as skill_imports, SkillImportPreview,
27+
};
2528
use openbitfun_core::agentic::tools::implementations::skills::{
2629
resolver::resolve_skill_default_enabled_for_mode, ModeSkillInfo, SkillData, SkillInfo,
2730
SkillLocation, SkillRegistry, SkillScanReport,
@@ -86,6 +89,12 @@ fn ensure_skill_can_be_deleted(skill: &SkillInfo) -> Result<(), String> {
8689

8790
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
8891
pub struct SkillValidationResult {
92+
#[serde(
93+
default,
94+
rename = "importPreview",
95+
skip_serializing_if = "Option::is_none"
96+
)]
97+
pub import_preview: Option<SkillImportPreview>,
8998
pub valid: bool,
9099
pub name: Option<String>,
91100
pub description: Option<String>,
@@ -574,7 +583,7 @@ pub async fn get_skill_configs(
574583
};
575584
object.insert(
576585
"importOperationsVersion".into(),
577-
serde_json::json!(if supported { 2 } else { 0 }),
586+
serde_json::json!(if supported { 3 } else { 0 }),
578587
);
579588
}
580589
Ok(response)
@@ -885,14 +894,58 @@ pub async fn reset_mode_skill_selection(
885894
))
886895
}
887896

897+
async fn resolve_external_skill_import_source(
898+
source_path: &str,
899+
source_key: &str,
900+
workspace: Option<&Path>,
901+
) -> Result<SkillInfo, String> {
902+
if let Some(root) = workspace {
903+
if is_remote_path(&root.to_string_lossy()).await {
904+
return Err("External Skill import into remote workspaces is not supported".into());
905+
}
906+
}
907+
let source = SkillRegistry::global()
908+
.find_skill_by_key_for_workspace(source_key, workspace)
909+
.await
910+
.ok_or("skill_import_stale: External Skill source changed; refresh before importing")?;
911+
if tokio::fs::canonicalize(&source.path)
912+
.await
913+
.map_err(|error| error.to_string())?
914+
!= tokio::fs::canonicalize(source_path)
915+
.await
916+
.map_err(|error| error.to_string())?
917+
{
918+
return Err("External Skill path does not match the selected source".into());
919+
}
920+
Ok(source)
921+
}
922+
888923
#[tauri::command]
889-
pub async fn validate_skill_path(path: String) -> Result<SkillValidationResult, String> {
924+
pub async fn validate_skill_path(
925+
path: String,
926+
source_key: Option<String>,
927+
workspace_path: Option<String>,
928+
) -> Result<SkillValidationResult, String> {
890929
use std::path::Path;
930+
if let Some(source_key) = source_key {
931+
let workspace = workspace_root_from_input(workspace_path.as_deref());
932+
let source =
933+
resolve_external_skill_import_source(&path, &source_key, workspace.as_deref()).await?;
934+
let preview = skill_imports::preview_import(source).await?;
935+
return Ok(SkillValidationResult {
936+
valid: true,
937+
name: Some(preview.name.clone()),
938+
description: Some(preview.description.clone()),
939+
error: None,
940+
import_preview: Some(preview),
941+
});
942+
}
891943

892944
let skill_path = Path::new(&path);
893945

894946
if !skill_path.exists() {
895947
return Ok(SkillValidationResult {
948+
import_preview: None,
896949
valid: false,
897950
name: None,
898951
description: None,
@@ -902,6 +955,7 @@ pub async fn validate_skill_path(path: String) -> Result<SkillValidationResult,
902955

903956
if !skill_path.is_dir() {
904957
return Ok(SkillValidationResult {
958+
import_preview: None,
905959
valid: false,
906960
name: None,
907961
description: None,
@@ -912,6 +966,7 @@ pub async fn validate_skill_path(path: String) -> Result<SkillValidationResult,
912966
let skill_md_path = skill_path.join("SKILL.md");
913967
if !skill_md_path.exists() {
914968
return Ok(SkillValidationResult {
969+
import_preview: None,
915970
valid: false,
916971
name: None,
917972
description: None,
@@ -923,12 +978,14 @@ pub async fn validate_skill_path(path: String) -> Result<SkillValidationResult,
923978
Ok(content) => {
924979
match SkillData::from_markdown(path.clone(), &content, SkillLocation::User, false) {
925980
Ok(data) => Ok(SkillValidationResult {
981+
import_preview: None,
926982
valid: true,
927983
name: Some(data.name),
928984
description: Some(data.description),
929985
error: None,
930986
}),
931987
Err(e) => Ok(SkillValidationResult {
988+
import_preview: None,
932989
valid: false,
933990
name: None,
934991
description: None,
@@ -937,6 +994,7 @@ pub async fn validate_skill_path(path: String) -> Result<SkillValidationResult,
937994
}
938995
}
939996
Err(e) => Ok(SkillValidationResult {
997+
import_preview: None,
940998
valid: false,
941999
name: None,
9421000
description: None,
@@ -953,30 +1011,16 @@ pub async fn add_skill(
9531011
workspace_path: Option<String>,
9541012
source_key: Option<String>,
9551013
target_name: Option<String>,
1014+
expected_source_fingerprint: Option<String>,
9561015
) -> Result<String, String> {
9571016
if let Some(source_key) = source_key {
9581017
if !matches!(level.as_str(), "user" | "project") {
9591018
return Err("Invalid Skill target scope".into());
9601019
}
9611020
let workspace = workspace_root_from_input(workspace_path.as_deref());
962-
if let Some(root) = &workspace {
963-
if is_remote_path(&root.to_string_lossy()).await {
964-
return Err("External Skill import into remote workspaces is not supported".into());
965-
}
966-
}
967-
let source = SkillRegistry::global()
968-
.find_skill_by_key_for_workspace(&source_key, workspace.as_deref())
969-
.await
970-
.ok_or("External Skill source changed; refresh before importing")?;
971-
if tokio::fs::canonicalize(&source.path)
972-
.await
973-
.map_err(|error| error.to_string())?
974-
!= tokio::fs::canonicalize(&source_path)
975-
.await
976-
.map_err(|error| error.to_string())?
977-
{
978-
return Err("External Skill path does not match the selected source".into());
979-
}
1021+
let source =
1022+
resolve_external_skill_import_source(&source_path, &source_key, workspace.as_deref())
1023+
.await?;
9801024
let paths = get_path_manager_arc();
9811025
let target = if level == "project" {
9821026
paths
@@ -985,19 +1029,22 @@ pub async fn add_skill(
9851029
} else {
9861030
paths.user_skills_dir()
9871031
};
988-
openbitfun_core::agentic::tools::implementations::skills::registry::imports::import_copy_as(
989-
source, target, target_name,
1032+
skill_imports::import_copy_as_reviewed(
1033+
source,
1034+
target,
1035+
target_name,
1036+
expected_source_fingerprint,
9901037
)
9911038
.await?;
9921039
SkillRegistry::global()
9931040
.refresh_for_workspace(workspace.as_deref())
9941041
.await;
9951042
return Ok("External Skill imported successfully".into());
9961043
}
997-
if target_name.is_some() {
998-
return Err("Renaming an imported Skill requires its source identity".into());
1044+
if target_name.is_some() || expected_source_fingerprint.is_some() {
1045+
return Err("Reviewed or renamed Skill imports require their source identity".into());
9991046
}
1000-
let validation = validate_skill_path(source_path.clone()).await?;
1047+
let validation = validate_skill_path(source_path.clone(), None, None).await?;
10011048
if !validation.valid {
10021049
return Err(validation.error.unwrap_or("Invalid skill path".to_string()));
10031050
}
@@ -1152,7 +1199,7 @@ pub async fn delete_skill(
11521199
let skill_path = std::path::PathBuf::from(&skill_info.path);
11531200

11541201
if let Some(expected) = expected_import_id {
1155-
openbitfun_core::agentic::tools::implementations::skills::registry::imports::remove_imported_copy(&skill_path, &expected).await?;
1202+
skill_imports::remove_imported_copy(&skill_path, &expected).await?;
11561203
} else if skill_path.exists() {
11571204
if let Err(e) = tokio::fs::remove_dir_all(&skill_path).await {
11581205
return Err(format!("Failed to delete skill folder: {}", e));
@@ -1177,6 +1224,35 @@ mod tests {
11771224
use std::future;
11781225
use tokio::time::Duration;
11791226

1227+
#[test]
1228+
fn skill_validation_reads_and_round_trips_legacy_payloads() {
1229+
let old = serde_json::json!({ "valid": true, "name": "demo", "description": "existing", "error": null });
1230+
let parsed: super::SkillValidationResult = serde_json::from_value(old.clone()).unwrap();
1231+
assert!(parsed.import_preview.is_none());
1232+
assert_eq!(serde_json::to_value(parsed).unwrap(), old);
1233+
let new = serde_json::json!({ "valid": true, "name": "demo", "description": "existing", "error": null,
1234+
"importPreview": { "fingerprint": "reviewed", "fileCount": 2, "name": "demo", "description": "existing" } });
1235+
let parsed: super::SkillValidationResult = serde_json::from_value(new.clone()).unwrap();
1236+
assert_eq!(serde_json::to_value(parsed).unwrap(), new);
1237+
}
1238+
1239+
#[tokio::test]
1240+
async fn legacy_skill_path_validation_remains_available_without_source_identity() {
1241+
let temp = tempfile::tempdir().unwrap();
1242+
std::fs::write(
1243+
temp.path().join("SKILL.md"),
1244+
"---\nname: demo\ndescription: Existing\n---\nBody",
1245+
)
1246+
.unwrap();
1247+
let result =
1248+
super::validate_skill_path(temp.path().to_string_lossy().into_owned(), None, None)
1249+
.await
1250+
.unwrap();
1251+
assert!(result.valid);
1252+
assert!(result.import_preview.is_none());
1253+
assert_eq!(result.name.as_deref(), Some("demo"));
1254+
}
1255+
11801256
#[test]
11811257
fn skill_scan_response_preserves_legacy_arrays_and_opt_in_diagnostics() {
11821258
let report = super::SkillScanReport {

‎src/crates/adapters/claude-code-adapter/src/agent_source.rs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ pub struct ClaudeCodeSubagentProviderOptions {
5656
impl ClaudeCodeSubagentProviderOptions {
5757
pub fn from_environment() -> Self {
5858
Self {
59-
user_claude_dir: dirs::home_dir()
60-
.unwrap_or_else(|| PathBuf::from("."))
61-
.join(".claude"),
59+
user_claude_dir: crate::ClaudeCodeInstructionSourceOptions::from_environment()
60+
.config_dir
61+
.unwrap_or_default(),
6262
project_root_override: None,
6363
project_config_enabled: true,
6464
}
@@ -140,6 +140,13 @@ impl ExternalSubagentSourceProvider for ClaudeCodeSubagentProvider {
140140
&self,
141141
input: &ExternalSubagentDiscoveryInput,
142142
) -> Result<ExternalSubagentProviderSnapshot, ExternalSourceProviderError> {
143+
if !self.options.user_claude_dir.is_absolute() {
144+
return Err(ExternalSourceProviderError::new(
145+
"claude.config_root_invalid",
146+
"Claude Code configuration directory must be absolute",
147+
false,
148+
));
149+
}
143150
if input
144151
.context
145152
.workspace_root

‎src/crates/adapters/claude-code-adapter/src/command_source.rs‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ pub struct ClaudeCodeCommandProviderOptions {
3939
impl ClaudeCodeCommandProviderOptions {
4040
pub fn from_environment() -> Self {
4141
Self {
42-
user_claude_dir: dirs::home_dir()
43-
.unwrap_or_else(|| PathBuf::from("."))
44-
.join(".claude"),
42+
user_claude_dir: crate::ClaudeCodeInstructionSourceOptions::from_environment()
43+
.config_dir
44+
.unwrap_or_default(),
4545
project_root_override: None,
4646
project_config_enabled: true,
4747
}
@@ -113,6 +113,13 @@ impl PromptCommandSourceProvider for ClaudeCodeCommandProvider {
113113
&self,
114114
context: &ExternalSourceContext,
115115
) -> Result<PromptCommandProviderSnapshot, ExternalSourceProviderError> {
116+
if !self.options.user_claude_dir.is_absolute() {
117+
return Err(ExternalSourceProviderError::new(
118+
"claude.config_root_invalid",
119+
"Claude Code configuration directory must be absolute",
120+
false,
121+
));
122+
}
116123
if context
117124
.workspace_root
118125
.as_ref()
@@ -345,7 +352,9 @@ impl PromptCommandSourceProvider for ClaudeCodeCommandProvider {
345352

346353
fn watch_roots(&self, context: &ExternalSourceContext) -> Vec<ExternalWatchRoot> {
347354
let mut roots = BTreeMap::new();
348-
roots.insert(self.options.user_claude_dir.clone(), true);
355+
if self.options.user_claude_dir.is_absolute() {
356+
roots.insert(self.options.user_claude_dir.clone(), true);
357+
}
349358
if self.options.project_config_enabled {
350359
if let Some(workspace) = &context.workspace_root {
351360
let project_root = self.project_root(workspace);

‎src/crates/adapters/claude-code-adapter/src/hook_source.rs‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,10 @@ pub struct ClaudeCodeHookProviderOptions {
6060

6161
impl ClaudeCodeHookProviderOptions {
6262
pub fn from_environment() -> Self {
63-
let user_settings_file = dirs::home_dir()
64-
.unwrap_or_else(|| PathBuf::from("."))
65-
.join(".claude/settings.json");
63+
let user_settings_file = crate::ClaudeCodeInstructionSourceOptions::from_environment()
64+
.config_dir
65+
.unwrap_or_default()
66+
.join("settings.json");
6667
Self {
6768
user_settings_file,
6869
project_root_override: None,
@@ -158,6 +159,13 @@ impl ExternalHookSourceProvider for ClaudeCodeHookProvider {
158159
&self,
159160
context: &ExternalSourceContext,
160161
) -> Result<ExternalHookProviderSnapshot, ExternalSourceProviderError> {
162+
if !self.options.user_settings_file.is_absolute() {
163+
return Err(ExternalSourceProviderError::new(
164+
"claude.config_root_invalid",
165+
"Claude Code configuration directory must be absolute",
166+
false,
167+
));
168+
}
161169
if context
162170
.workspace_root
163171
.as_ref()
@@ -253,6 +261,12 @@ impl ExternalHookSourceProvider for ClaudeCodeHookProvider {
253261
requested_source: &SourceKey,
254262
expected_catalog_content_version: &str,
255263
) -> Result<PreparedExternalHookImport, ExternalSourceProviderError> {
264+
if !self.options.user_settings_file.is_absolute() {
265+
return Err(import_error(
266+
"claude.config_root_invalid",
267+
"Claude Code configuration directory must be absolute",
268+
));
269+
}
256270
if requested_source.provider_id.as_str() != PROVIDER_ID {
257271
return Err(import_error(
258272
"claude.hook.import_provider_mismatch",

0 commit comments

Comments
 (0)