From bf0b82d80f1b99fdd16ffa1401a358f5239b5f8b Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Thu, 11 Jun 2026 21:32:52 +0800 Subject: [PATCH 1/7] feat(core): support user-scoped content targets --- crates/ov_cli/src/client.rs | 86 ++++++++++-- crates/ov_cli/src/commands/resources.rs | 15 +- crates/ov_cli/src/commands/skills.rs | 74 +++++++--- crates/ov_cli/src/handlers.rs | 15 ++ crates/ov_cli/src/main.rs | 131 +++++++++++++++--- .../scripts/debug-recall.mjs | 2 +- openviking/async_client.py | 6 + openviking/client/local.py | 6 + openviking/core/content_targets.py | 52 +++++++ openviking/core/directories.py | 5 + openviking/core/identifiers.py | 53 +++++++ openviking/core/namespace.py | 99 ++++++++++--- openviking/core/peer_id.py | 20 ++- openviking/core/retrieval_targets.py | 76 ++++++---- openviking/core/uri_validation.py | 64 ++++++++- openviking/parse/tree_builder.py | 31 +---- openviking/server/mcp_endpoint.py | 1 + openviking/server/routers/resources.py | 21 +-- openviking/server/routers/skills.py | 85 ++++++++---- openviking/service/resource_service.py | 83 +++++------ openviking/storage/ovpack/operations.py | 9 +- openviking/storage/viking_fs.py | 60 ++++---- openviking/sync_client.py | 13 +- openviking/utils/skill_processor.py | 56 +++++++- openviking_cli/client/base.py | 3 + openviking_cli/client/http.py | 6 + openviking_cli/client/sync_http.py | 13 +- openviking_cli/session/user_id.py | 50 ++----- tests/cli/test_user_identifier.py | 7 + tests/client/test_http_client_config.py | 8 +- tests/server/test_api_resources.py | 46 ++++++ tests/server/test_api_skills.py | 65 +++++++++ tests/server/test_peer_id_compat.py | 69 +++++---- .../memory/test_memory_isolation_handler.py | 62 ++++----- .../memory/test_memory_react_system_prompt.py | 4 +- .../memory/test_memory_timestamp_parsing.py | 40 +++--- tests/session/test_session_commit.py | 8 +- tests/session/test_session_messages.py | 6 +- .../test_tool_result_externalization.py | 4 +- .../unit/test_namespace_uri_classification.py | 85 +++++++++++- 40 files changed, 1155 insertions(+), 384 deletions(-) create mode 100644 openviking/core/content_targets.py create mode 100644 openviking/core/identifiers.py diff --git a/crates/ov_cli/src/client.rs b/crates/ov_cli/src/client.rs index 6f228e68fa..c1ee22fe5e 100644 --- a/crates/ov_cli/src/client.rs +++ b/crates/ov_cli/src/client.rs @@ -625,6 +625,9 @@ impl HttpClient { pub async fn add_skill( &self, data: &str, + to: Option, + parent: Option, + parent_auto_create: Option, wait: bool, timeout: Option, show_progress: bool, @@ -632,6 +635,31 @@ impl HttpClient { source_metadata: Option, ) -> Result { let path_obj = Path::new(data); + let (effective_parent, create_parent) = match (parent, parent_auto_create) { + (Some(p), None) => (Some(p), false), + (None, Some(p)) => (Some(p), true), + (None, None) => (None, false), + (Some(_), Some(_)) => { + return Err(Error::Client( + "Specify only one of --parent or --parent-auto-create.".to_string(), + )); + } + }; + if to.is_some() && effective_parent.is_some() { + return Err(Error::Client( + "Specify only one of --to, --parent, or --parent-auto-create.".to_string(), + )); + } + + let build_body = |base: serde_json::Value| { + let mut body = base; + if create_parent { + body.as_object_mut() + .expect("add_skill request body must be an object") + .insert("create_parent".to_string(), serde_json::Value::Bool(true)); + } + body + }; if path_obj.exists() { if path_obj.is_dir() { @@ -647,11 +675,13 @@ impl HttpClient { self.upload_temp_file(zip_file.path()).await? }; - let mut body = serde_json::json!({ + let mut body = build_body(serde_json::json!({ "temp_file_id": temp_file_id, + "to": to, + "parent": effective_parent, "wait": wait, "timeout": timeout, - }); + })); if let Some(source_metadata) = source_metadata.clone() { body["source_metadata"] = source_metadata; } @@ -668,11 +698,13 @@ impl HttpClient { self.upload_temp_file(path_obj).await? }; - let mut body = serde_json::json!({ + let mut body = build_body(serde_json::json!({ "temp_file_id": temp_file_id, + "to": to, + "parent": effective_parent, "wait": wait, "timeout": timeout, - }); + })); if let Some(source_metadata) = source_metadata.clone() { body["source_metadata"] = source_metadata; } @@ -682,22 +714,26 @@ impl HttpClient { .post_with_timeout("/api/v1/skills", &body, dynamic_timeout) .await } else { - let mut body = serde_json::json!({ + let mut body = build_body(serde_json::json!({ "data": data, + "to": to, + "parent": effective_parent, "wait": wait, "timeout": timeout, - }); + })); if let Some(source_metadata) = source_metadata.clone() { body["source_metadata"] = source_metadata; } self.post("/api/v1/skills", &body).await } } else { - let mut body = serde_json::json!({ + let mut body = build_body(serde_json::json!({ "data": data, + "to": to, + "parent": effective_parent, "wait": wait, "timeout": timeout, - }); + })); if let Some(source_metadata) = source_metadata { body["source_metadata"] = source_metadata; } @@ -705,8 +741,15 @@ impl HttpClient { } } - pub async fn skills_list(&self, node_limit: i32) -> Result { - let params = vec![("node_limit".to_string(), node_limit.to_string())]; + pub async fn skills_list( + &self, + node_limit: i32, + root_uri: Option<&str>, + ) -> Result { + let mut params = vec![("node_limit".to_string(), node_limit.to_string())]; + if let Some(root_uri) = root_uri { + params.push(("root_uri".to_string(), root_uri.to_string())); + } self.get("/api/v1/skills", ¶ms).await } @@ -717,6 +760,7 @@ impl HttpClient { include_files: bool, include_source: bool, level: Option, + root_uri: Option<&str>, ) -> Result { let path = format!("/api/v1/skills/{}", name); let mut params = vec![ @@ -727,6 +771,9 @@ impl HttpClient { if let Some(level) = level { params.push(("level".to_string(), level.to_string())); } + if let Some(root_uri) = root_uri { + params.push(("root_uri".to_string(), root_uri.to_string())); + } self.get(&path, ¶ms).await } @@ -736,9 +783,11 @@ impl HttpClient { node_limit: i32, threshold: Option, level: Option>, + root_uri: Option<&str>, ) -> Result { let body = serde_json::json!({ "query": query, + "root_uri": root_uri, "limit": node_limit, "score_threshold": threshold, "level": level, @@ -809,6 +858,7 @@ impl HttpClient { show_progress: bool, verbose: bool, source_metadata: Option, + root_uri: Option<&str>, ) -> Result { let endpoint = format!("/api/v1/skills/{}", name); let path_obj = Path::new(data); @@ -828,6 +878,7 @@ impl HttpClient { }; let mut body = serde_json::json!({ "temp_file_id": temp_file_id, + "root_uri": root_uri, "wait": wait, "timeout": timeout, }); @@ -844,6 +895,7 @@ impl HttpClient { }; let mut body = serde_json::json!({ "temp_file_id": temp_file_id, + "root_uri": root_uri, "wait": wait, "timeout": timeout, }); @@ -854,6 +906,7 @@ impl HttpClient { } else { let mut body = serde_json::json!({ "data": data, + "root_uri": root_uri, "wait": wait, "timeout": timeout, }); @@ -865,6 +918,7 @@ impl HttpClient { } else { let mut body = serde_json::json!({ "data": data, + "root_uri": root_uri, "wait": wait, "timeout": timeout, }); @@ -875,9 +929,17 @@ impl HttpClient { } } - pub async fn skill_remove(&self, name: &str) -> Result { + pub async fn skill_remove( + &self, + name: &str, + root_uri: Option<&str>, + ) -> Result { let path = format!("/api/v1/skills/{}", name); - self.delete(&path, &[]).await + let mut params = Vec::new(); + if let Some(root_uri) = root_uri { + params.push(("root_uri".to_string(), root_uri.to_string())); + } + self.delete(&path, ¶ms).await } // ============ Task Methods ============ diff --git a/crates/ov_cli/src/commands/resources.rs b/crates/ov_cli/src/commands/resources.rs index ca9c404cdc..69698dc3fc 100644 --- a/crates/ov_cli/src/commands/resources.rs +++ b/crates/ov_cli/src/commands/resources.rs @@ -58,6 +58,9 @@ pub async fn add_resource( pub async fn add_skill( client: &HttpClient, data: &str, + to: Option, + parent: Option, + parent_auto_create: Option, wait: bool, timeout: Option, show_progress: bool, @@ -66,7 +69,17 @@ pub async fn add_skill( compact: bool, ) -> Result<()> { let result = client - .add_skill(data, wait, timeout, show_progress, verbose, None) + .add_skill( + data, + to, + parent, + parent_auto_create, + wait, + timeout, + show_progress, + verbose, + None, + ) .await?; if !wait && matches!(format, OutputFormat::Table) { diff --git a/crates/ov_cli/src/commands/skills.rs b/crates/ov_cli/src/commands/skills.rs index 855a82c598..06e2601312 100644 --- a/crates/ov_cli/src/commands/skills.rs +++ b/crates/ov_cli/src/commands/skills.rs @@ -117,6 +117,9 @@ pub async fn add( let result = client .add_skill( &target.data, + None, + None, + None, wait, None, show_progress, @@ -152,11 +155,12 @@ pub async fn add( pub async fn list( client: &HttpClient, + root_uri: Option<&str>, node_limit: i32, output_format: OutputFormat, compact: bool, ) -> Result<()> { - let result = client.skills_list(node_limit).await?; + let result = client.skills_list(node_limit, root_uri).await?; output_success(result, output_format, compact); Ok(()) } @@ -164,6 +168,7 @@ pub async fn list( pub async fn show( client: &HttpClient, name: &str, + root_uri: Option<&str>, level: Option, include_files: bool, include_source: bool, @@ -172,7 +177,14 @@ pub async fn show( ) -> Result<()> { let include_content = level.is_none() || level == Some(2); let mut result = client - .skill_show(name, include_content, include_files, include_source, level) + .skill_show( + name, + include_content, + include_files, + include_source, + level, + root_uri, + ) .await?; if let Some(level) = level { filter_skill_show_level(&mut result, level); @@ -184,6 +196,7 @@ pub async fn show( pub async fn find( client: &HttpClient, query: &str, + root_uri: Option<&str>, node_limit: i32, threshold: Option, level: Option>, @@ -191,7 +204,7 @@ pub async fn find( compact: bool, ) -> Result<()> { let result = client - .skill_find(query, node_limit, threshold, level) + .skill_find(query, node_limit, threshold, level, root_uri) .await?; output_success(result, output_format, compact); Ok(()) @@ -199,6 +212,7 @@ pub async fn find( pub async fn update( client: &HttpClient, + root_uri: Option<&str>, skill_names: Vec, wait: bool, yes: bool, @@ -206,7 +220,7 @@ pub async fn update( compact: bool, ) -> Result<()> { let update_all = skill_names.is_empty(); - let names = resolve_installed_skill_names(client, skill_names).await?; + let names = resolve_installed_skill_names(client, root_uri, skill_names).await?; if names.is_empty() { output_message_result( serde_json::json!({ "updated": [], "total": 0 }), @@ -229,7 +243,8 @@ pub async fn update( let mut updated = Vec::new(); let mut skipped = Vec::new(); for name in names { - let update_target = match resolve_update_target(client, &name, !update_all).await { + let update_target = match resolve_update_target(client, root_uri, &name, !update_all).await + { Ok(target) => target, Err(error) if update_all => { skipped.push(json!({ @@ -250,6 +265,7 @@ pub async fn update( false, false, source_metadata, + root_uri, ) .await?; updated.push(result); @@ -271,6 +287,7 @@ pub async fn update( pub async fn remove( client: &HttpClient, + root_uri: Option<&str>, skill_names: Vec, all: bool, yes: bool, @@ -284,7 +301,7 @@ pub async fn remove( } let requested_names = normalize_skill_names(skill_names)?; let names = if all { - resolve_installed_skill_names(client, Vec::new()).await? + resolve_installed_skill_names(client, root_uri, Vec::new()).await? } else if !requested_names.is_empty() { requested_names } else { @@ -293,7 +310,7 @@ pub async fn remove( "Specify at least one skill name, or pass --all.".to_string(), )); } - match prompt_remove_skill_selection(client).await? { + match prompt_remove_skill_selection(client, root_uri).await? { Some(selected) => selected, None => { output_message_result( @@ -328,7 +345,7 @@ pub async fn remove( let removed_names = names.clone(); let mut removed = Vec::new(); for name in names { - removed.push(client.skill_remove(&name).await?); + removed.push(client.skill_remove(&name, root_uri).await?); } let total = removed.len(); output_message_result( @@ -800,7 +817,9 @@ fn normalize_git_subdir(subdir: &Path) -> Result { } if normalized.as_os_str().is_empty() { - return Err(Error::Parse("Git source metadata missing subdir".to_string())); + return Err(Error::Parse( + "Git source metadata missing subdir".to_string(), + )); } Ok(normalized) @@ -1132,10 +1151,11 @@ fn skill_target_label(target: &AddTarget) -> String { async fn resolve_update_target( client: &HttpClient, + root_uri: Option<&str>, name: &str, allow_prompt: bool, ) -> Result { - if let Some(record) = read_skill_source_record(client, name).await? { + if let Some(record) = read_skill_source_record(client, root_uri, name).await? { return update_target_from_record(&record, name, allow_prompt); } @@ -1153,9 +1173,12 @@ async fn resolve_update_target( async fn read_skill_source_record( client: &HttpClient, + root_uri: Option<&str>, name: &str, ) -> Result> { - let result = client.skill_show(name, false, false, true, Some(0)).await?; + let result = client + .skill_show(name, false, false, true, Some(0), root_uri) + .await?; let Some(source) = result.get("source") else { return Ok(None); }; @@ -1330,6 +1353,7 @@ fn resolve_named_skill_dir(root: &Path, name: &str) -> Result { async fn resolve_installed_skill_names( client: &HttpClient, + root_uri: Option<&str>, requested: Vec, ) -> Result> { let requested = normalize_skill_names(requested)?; @@ -1337,15 +1361,18 @@ async fn resolve_installed_skill_names( return Ok(requested); } - Ok(list_installed_skills(client) + Ok(list_installed_skills(client, root_uri) .await? .into_iter() .map(|skill| skill.name) .collect()) } -async fn list_installed_skills(client: &HttpClient) -> Result> { - let result = client.skills_list(10000).await?; +async fn list_installed_skills( + client: &HttpClient, + root_uri: Option<&str>, +) -> Result> { + let result = client.skills_list(10000, root_uri).await?; let skills = result .get("skills") .and_then(Value::as_array) @@ -1365,8 +1392,11 @@ async fn list_installed_skills(client: &HttpClient) -> Result Result>> { - let skills = list_installed_skills(client).await?; +async fn prompt_remove_skill_selection( + client: &HttpClient, + root_uri: Option<&str>, +) -> Result>> { + let skills = list_installed_skills(client, root_uri).await?; if skills.is_empty() { return Ok(Some(Vec::new())); } @@ -1936,7 +1966,11 @@ mod tests { let error = update_target_from_record(&record, "local-skill", false) .expect_err("local source should not be trusted for non-interactive update"); - assert!(error.to_string().contains("server-recorded local paths are not trusted")); + assert!( + error + .to_string() + .contains("server-recorded local paths are not trusted") + ); } #[test] @@ -1954,7 +1988,11 @@ mod tests { let error = prepare_source_from_git_record(&record) .err() .expect("parent dir subdir should be rejected before clone"); - assert!(error.to_string().contains("parent directory components are not allowed")); + assert!( + error + .to_string() + .contains("parent directory components are not allowed") + ); } #[test] diff --git a/crates/ov_cli/src/handlers.rs b/crates/ov_cli/src/handlers.rs index f1e9d10390..6242ebff05 100644 --- a/crates/ov_cli/src/handlers.rs +++ b/crates/ov_cli/src/handlers.rs @@ -110,14 +110,29 @@ pub async fn handle_add_resource( pub async fn handle_add_skill( data: String, + to: Option, + parent: Option, + parent_auto_create: Option, wait: bool, timeout: Option, ctx: CliContext, ) -> Result<()> { + let exclusive_count = usize::from(to.is_some()) + + usize::from(parent.is_some()) + + usize::from(parent_auto_create.is_some()); + if exclusive_count > 1 { + return Err(Error::Client( + "Specify only one of --to, --parent, or --parent-auto-create.".to_string(), + )); + } + let client = ctx.get_client(); commands::resources::add_skill( &client, &data, + to, + parent, + parent_auto_create, wait, timeout, ctx.should_show_progress(), diff --git a/crates/ov_cli/src/main.rs b/crates/ov_cli/src/main.rs index 5ec672ca6e..5b472c0bfc 100644 --- a/crates/ov_cli/src/main.rs +++ b/crates/ov_cli/src/main.rs @@ -268,6 +268,15 @@ enum Commands { AddSkill { /// Skill directory, SKILL.md, or raw content data: String, + /// Exact target URI (must end with the skill name) (cannot be used with --parent) + #[arg(long)] + to: Option, + /// Target parent skills URI (must already exist and be a directory) (cannot be used with --to) + #[arg(long)] + parent: Option, + /// Target parent skills URI (create parent directory if it does not exist) (cannot be used with --to or --parent) + #[arg(short = 'p', long = "parent-auto-create")] + parent_auto_create: Option, /// Wait until processing is complete #[arg(long)] wait: bool, @@ -879,6 +888,9 @@ enum SkillCommands { /// List installed agent skills #[command(alias = "ls")] List { + /// Skills root URI to manage + #[arg(long = "root-uri")] + root_uri: Option, /// Maximum number of skills to list #[arg( short = 'n', @@ -892,6 +904,9 @@ enum SkillCommands { Find { /// Search query query: String, + /// Skills root URI to search + #[arg(long = "root-uri")] + root_uri: Option, /// Maximum number of results #[arg( short = 'n', @@ -911,6 +926,9 @@ enum SkillCommands { Show { /// Skill name name: String, + /// Skills root URI containing the skill + #[arg(long = "root-uri")] + root_uri: Option, /// Detail level to show (0=abstract, 1=overview, 2=SKILL.md content) #[arg(short = 'L', long = "level", value_parser = clap::value_parser!(i32).range(0..=2))] level: Option, @@ -929,6 +947,9 @@ enum SkillCommands { }, /// Update installed skills from their recorded source Update { + /// Skills root URI to update from + #[arg(long = "root-uri")] + root_uri: Option, /// Skill name(s) to update; omit to update all installed skills skills: Vec, /// Wait until processing is complete @@ -941,6 +962,9 @@ enum SkillCommands { /// Remove installed skills #[command(alias = "rm", alias = "delete")] Remove { + /// Skills root URI to remove from + #[arg(long = "root-uri")] + root_uri: Option, /// Skill name(s) to remove skills: Vec, /// Remove all installed skills @@ -2189,13 +2213,17 @@ async fn main() { } Commands::AddSkill { data, + to, + parent, + parent_auto_create, wait, timeout, upload_options, } => { let ctx = ctx.with_upload_options(upload_options.merged_with_legacy(legacy_upload_options)); - handlers::handle_add_skill(data, wait, timeout, ctx).await + handlers::handle_add_skill(data, to, parent, parent_auto_create, wait, timeout, ctx) + .await } Commands::Skills { action } => match action { SkillCommands::Add { @@ -2220,12 +2248,23 @@ async fn main() { ) .await } - SkillCommands::List { node_limit } => { + SkillCommands::List { + root_uri, + node_limit, + } => { let client = ctx.get_client(); - commands::skills::list(&client, node_limit, ctx.output_format, ctx.compact).await + commands::skills::list( + &client, + root_uri.as_deref(), + node_limit, + ctx.output_format, + ctx.compact, + ) + .await } SkillCommands::Find { query, + root_uri, node_limit, threshold, level, @@ -2234,6 +2273,7 @@ async fn main() { commands::skills::find( &client, &query, + root_uri.as_deref(), node_limit, threshold, level, @@ -2244,6 +2284,7 @@ async fn main() { } SkillCommands::Show { name, + root_uri, level, files, source, @@ -2259,6 +2300,7 @@ async fn main() { commands::skills::show( &client, &name, + root_uri.as_deref(), level, files, source, @@ -2267,15 +2309,41 @@ async fn main() { ) .await } - SkillCommands::Update { skills, wait, yes } => { + SkillCommands::Update { + root_uri, + skills, + wait, + yes, + } => { let client = ctx.get_client(); - commands::skills::update(&client, skills, wait, yes, ctx.output_format, ctx.compact) - .await + commands::skills::update( + &client, + root_uri.as_deref(), + skills, + wait, + yes, + ctx.output_format, + ctx.compact, + ) + .await } - SkillCommands::Remove { skills, all, yes } => { + SkillCommands::Remove { + root_uri, + skills, + all, + yes, + } => { let client = ctx.get_client(); - commands::skills::remove(&client, skills, all, yes, ctx.output_format, ctx.compact) - .await + commands::skills::remove( + &client, + root_uri.as_deref(), + skills, + all, + yes, + ctx.output_format, + ctx.compact, + ) + .await } SkillCommands::Validate { path, strict } => { let client = ctx.get_client(); @@ -2598,12 +2666,12 @@ mod tests { #[test] fn cli_parses_find_peer_id() { - let cli = Cli::try_parse_from(["ov", "find", "invoice", "--peer-id", "web:visitor:alice"]) + let cli = Cli::try_parse_from(["ov", "find", "invoice", "--peer-id", "web-visitor-alice"]) .expect("find peer id should parse"); match cli.command { Commands::Find { peer_id, .. } => { - assert_eq!(peer_id.as_deref(), Some("web:visitor:alice")); + assert_eq!(peer_id.as_deref(), Some("web-visitor-alice")); } _ => panic!("expected find command"), } @@ -2612,12 +2680,12 @@ mod tests { #[test] fn cli_parses_search_peer_id() { let cli = - Cli::try_parse_from(["ov", "search", "invoice", "--peer-id", "web:visitor:alice"]) + Cli::try_parse_from(["ov", "search", "invoice", "--peer-id", "web-visitor-alice"]) .expect("search peer id should parse"); match cli.command { Commands::Search { peer_id, .. } => { - assert_eq!(peer_id.as_deref(), Some("web:visitor:alice")); + assert_eq!(peer_id.as_deref(), Some("web-visitor-alice")); } _ => panic!("expected search command"), } @@ -3047,8 +3115,15 @@ mod tests { .expect("skills list should parse"); match list.command { Commands::Skills { - action: SkillCommands::List { node_limit }, - } => assert_eq!(node_limit, 25), + action: + SkillCommands::List { + node_limit, + root_uri, + }, + } => { + assert!(root_uri.is_none()); + assert_eq!(node_limit, 25); + } _ => panic!("expected skills list"), } @@ -3145,6 +3220,7 @@ mod tests { Commands::Skills { action: SkillCommands::Show { + root_uri, level, files, source, @@ -3152,6 +3228,7 @@ mod tests { .. }, } => { + assert!(root_uri.is_none()); assert_eq!(level, Some(2)); assert!(files); assert!(source); @@ -3160,6 +3237,25 @@ mod tests { _ => panic!("expected skills show"), } + let show_with_root_uri = Cli::try_parse_from([ + "ov", + "skills", + "show", + "code-review", + "--root-uri", + "viking://user/default/peers/alice/skills", + ]) + .expect("skills show root_uri should parse"); + match show_with_root_uri.command { + Commands::Skills { + action: SkillCommands::Show { root_uri, .. }, + } => assert_eq!( + root_uri.as_deref(), + Some("viking://user/default/peers/alice/skills") + ), + _ => panic!("expected skills show"), + } + let show_global_output = Cli::try_parse_from(["ov", "skills", "show", "code-review", "-o", "json"]) .expect("skills show should accept global -o after the subcommand"); @@ -3169,7 +3265,10 @@ mod tests { .expect("skills remove --yes should parse"); match remove.command { Commands::Skills { - action: SkillCommands::Remove { skills, yes, all }, + action: + SkillCommands::Remove { + skills, yes, all, .. + }, } => { assert_eq!(skills, vec!["foo", "bar"]); assert!(yes); diff --git a/examples/claude-code-memory-plugin/scripts/debug-recall.mjs b/examples/claude-code-memory-plugin/scripts/debug-recall.mjs index e6479c1d54..1fb8f20732 100755 --- a/examples/claude-code-memory-plugin/scripts/debug-recall.mjs +++ b/examples/claude-code-memory-plugin/scripts/debug-recall.mjs @@ -191,7 +191,7 @@ function postProcess(items, limit, threshold) { // --------------------------------------------------------------------------- const USER_RESERVED_DIRS = new Set(["memories"]); -const AGENT_RESERVED_DIRS = new Set(["memories", "skills", "instructions", "workspaces"]); +const AGENT_RESERVED_DIRS = new Set(["memories", "skills"]); const _spaceCache = {}; async function resolveScopeSpace(scope) { diff --git a/openviking/async_client.py b/openviking/async_client.py index 3312475832..a26a9aca14 100644 --- a/openviking/async_client.py +++ b/openviking/async_client.py @@ -336,6 +336,9 @@ async def summarize(self, resource_uris: Union[str, List[str]], **kwargs) -> Dic async def add_skill( self, data: Any, + to: Optional[str] = None, + parent: Optional[str] = None, + create_parent: bool = False, wait: bool = False, timeout: float = None, telemetry: TelemetryRequest = False, @@ -349,6 +352,9 @@ async def add_skill( await self._ensure_initialized() return await self._client.add_skill( data=data, + to=to, + parent=parent, + create_parent=create_parent, wait=wait, timeout=timeout, telemetry=telemetry, diff --git a/openviking/client/local.py b/openviking/client/local.py index 87c09901eb..f9a223123d 100644 --- a/openviking/client/local.py +++ b/openviking/client/local.py @@ -135,6 +135,9 @@ async def add_resource( async def add_skill( self, data: Any, + to: Optional[str] = None, + parent: Optional[str] = None, + create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, telemetry: TelemetryRequest = False, @@ -146,6 +149,9 @@ async def add_skill( fn=lambda: self._service.resources.add_skill( data=data, ctx=self._ctx, + to=to, + parent=parent, + create_parent=create_parent, wait=wait, timeout=timeout, ), diff --git a/openviking/core/content_targets.py b/openviking/core/content_targets.py new file mode 100644 index 0000000000..822fcfc431 --- /dev/null +++ b/openviking/core/content_targets.py @@ -0,0 +1,52 @@ +"""Shared target resolution for user-supplied content destinations.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import TYPE_CHECKING, Optional + +from openviking.core.path_variables import resolve_path_variables +from openviking.core.uri_validation import validate_optional_content_target_uri +from openviking_cli.exceptions import InvalidArgumentError + +if TYPE_CHECKING: + from openviking.server.identity import RequestContext + + +@dataclass(frozen=True) +class ContentTargetSpec: + """Canonical content destination fields for resource and skill writes.""" + + to: str = "" + parent: str = "" + create_parent: bool = False + + @classmethod + def from_fields( + cls, + *, + ctx: RequestContext, + kind: str, + to: Optional[str] = None, + parent: Optional[str] = None, + create_parent: bool = False, + ) -> "ContentTargetSpec": + resolved_to = resolve_path_variables(to) if to else None + resolved_parent = resolve_path_variables(parent) if parent else None + if (resolved_to or "").strip() and (resolved_parent or "").strip(): + raise InvalidArgumentError("Cannot specify both 'to' and 'parent' at the same time.") + return cls( + to=validate_optional_content_target_uri( + resolved_to, + ctx, + kind=kind, + field_name="to", + ), + parent=validate_optional_content_target_uri( + resolved_parent, + ctx, + kind=kind, + field_name="parent", + ), + create_parent=create_parent, + ) diff --git a/openviking/core/directories.py b/openviking/core/directories.py index 54d38a259d..96914fcbf3 100644 --- a/openviking/core/directories.py +++ b/openviking/core/directories.py @@ -103,6 +103,11 @@ class DirectoryDefinition: ), ], ), + DirectoryDefinition( + path="resources", + abstract="User-owned resource storage. Contains private documents and knowledge resources owned by the current User.", + overview="Use this directory for resources scoped to the current User. Project and document directories are created lazily as content is added.", + ), DirectoryDefinition( path="privacy", abstract="User privacy config root. Stores user-scoped sensitive configuration snapshots by category and target key.", diff --git a/openviking/core/identifiers.py b/openviking/core/identifiers.py new file mode 100644 index 0000000000..85d02bf1b9 --- /dev/null +++ b/openviking/core/identifiers.py @@ -0,0 +1,53 @@ +"""Shared account, user, and peer identifier validation.""" + +from __future__ import annotations + +import re +from typing import Optional + +_VALIDATION_PATTERN = re.compile(r"^[a-zA-Z0-9_.@-]+$") + + +def validate_identifier_part(part: str, part_name: str) -> str | None: + """Validate a single identifier path segment.""" + if not isinstance(part, str): + return f"{part_name} must be a string." + if not part: + return f"{part_name} is empty" + if part in {".", ".."}: + return f"{part_name} must not be '.' or '..'" + if not _VALIDATION_PATTERN.match(part): + return f"{part_name} must be alpha_numeric string." + if part.count("@") > 1: + return f"{part_name} must have at most one @." + return None + + +def normalize_identifier_part(part: Optional[str], part_name: str) -> Optional[str]: + """Normalize an optional identifier path segment and validate shared safety rules.""" + if part is None: + return None + if not isinstance(part, str): + raise ValueError(f"{part_name} must be a string.") + normalized = part.strip() + if not normalized: + return None + validation_error = validate_identifier_part(normalized, part_name) + if validation_error: + raise ValueError(validation_error) + return normalized + + +def validate_account_id(account_id: str) -> str | None: + """Validate an account id.""" + validation_error = validate_identifier_part(account_id, "account_id") + if validation_error: + return validation_error + if account_id.startswith("_"): + return "account_id cannot start with underscore _." + return None + + +def validate_user_id(user_id: str) -> str | None: + """Validate a user id.""" + return validate_identifier_part(user_id, "user_id") diff --git a/openviking/core/namespace.py b/openviking/core/namespace.py index d70e22ccd5..1981d1aa0f 100644 --- a/openviking/core/namespace.py +++ b/openviking/core/namespace.py @@ -5,23 +5,16 @@ from dataclasses import dataclass from typing import Optional +from openviking.core.identifiers import validate_user_id +from openviking.core.peer_id import normalize_peer_id from openviking.server.identity import RequestContext from openviking_cli.utils.uri import VikingURI -_USER_SHORTHAND_SEGMENTS = { - "memories", - "peers", - "privacy", - "sessions", - ".abstract.md", - ".overview.md", - "profile.md", - "skills", - "workspaces", -} _CONTENT_TYPES_BY_SCOPE = { - "user": {"memories": "memory", "skills": "skill"}, + "user": {"memories": "memory", "resources": "resource", "skills": "skill"}, } +_USER_RELATIVE_ROOT_SEGMENTS = frozenset({"peers", "privacy", "sessions"}) +_CONTENT_SEGMENT_BY_KIND = {"resource": "resources", "skill": "skills"} class NamespaceShapeError(ValueError): @@ -124,9 +117,9 @@ def _content_segment_index(parts: tuple[str, ...]) -> Optional[int]: return None if parts[1] in _CONTENT_TYPES_BY_SCOPE["user"]: return 1 - if len(parts) >= 4 and parts[1] == "peers" and parts[3] == "memories": + if len(parts) >= 4 and parts[1] == "peers" and parts[3] in _CONTENT_TYPES_BY_SCOPE["user"]: return 3 - if len(parts) >= 5 and parts[2] == "peers" and parts[4] == "memories": + if len(parts) >= 5 and parts[2] == "peers" and parts[4] in _CONTENT_TYPES_BY_SCOPE["user"]: return 4 if len(parts) >= 3 and parts[2] in _CONTENT_TYPES_BY_SCOPE["user"]: return 2 @@ -176,6 +169,11 @@ def canonical_session_uri(ctx: RequestContext, session_id: Optional[str] = None) return f"{root}/{session_id}" +def canonical_user_content_root(ctx: RequestContext, kind: str) -> str: + segment = _CONTENT_SEGMENT_BY_KIND[kind] + return f"{canonical_user_root(ctx)}/{segment}" + + def legacy_session_uri(session_id: Optional[str] = None) -> str: if not session_id: return "viking://session" @@ -246,6 +244,57 @@ def is_accessible(uri: str, ctx: RequestContext) -> bool: return True +def is_content_root_uri( + uri: str, + ctx: RequestContext, + *, + kind: str, +) -> bool: + try: + canonical_uri = canonicalize_uri(uri, ctx) + except (ValueError, NamespaceShapeError): + return False + parts = uri_parts(canonical_uri) + if kind == "resource" and parts == ["resources"]: + return True + classification = classify_uri(canonical_uri) + return ( + classification.context_type == kind + and classification.content_index is not None + and len(parts) == classification.content_index + 1 + ) + + +def is_content_namespace_root_uri(uri: str, ctx: RequestContext) -> bool: + try: + canonical_uri = canonicalize_uri(uri, ctx) + except (ValueError, NamespaceShapeError): + return False + parts = uri_parts(canonical_uri) + if parts == ["resources"]: + return True + classification = classify_uri(canonical_uri) + return ( + classification.content_index is not None and len(parts) == classification.content_index + 1 + ) + + +def _validate_peer_id_segments(parts: list[str]) -> None: + if len(parts) >= 3 and parts[0] == "user" and parts[1] == "peers": + _require_peer_id_segment(parts[2]) + return + if len(parts) >= 4 and parts[0] == "user" and parts[2] == "peers": + _require_peer_id_segment(parts[3]) + + +def _require_peer_id_segment(peer_id: str) -> None: + try: + if normalize_peer_id(peer_id) is None: + raise ValueError("peer_id must not be empty") + except ValueError as exc: + raise NamespaceShapeError(str(exc)) from exc + + def owner_fields_for_uri( uri: str, ctx: Optional[RequestContext] = None, @@ -299,7 +348,7 @@ def _resolve_user_uri( ) -> ResolvedNamespace: normalized = "viking://" + "/".join(parts) if len(parts) == 1: - if ctx is not None and getattr(ctx.role, "value", ctx.role) != "root": + if ctx is not None: return ResolvedNamespace( uri=canonical_user_root(ctx), scope="user", @@ -307,8 +356,7 @@ def _resolve_user_uri( ) return ResolvedNamespace(uri="viking://user", scope="user", is_container=True) - second = parts[1] - if second in _USER_SHORTHAND_SEGMENTS: + if _is_current_user_relative_uri(parts, ctx): if require_canonical: raise NamespaceShapeError(f"Shorthand user URI is not allowed here: {normalized}") if ctx is None: @@ -318,7 +366,11 @@ def _resolve_user_uri( "/".join([canonical_user_root(ctx)[len("viking://") :], *suffix]), ctx=ctx ) + second = parts[1] user_id = second + validation_error = validate_user_id(user_id) + if validation_error: + raise NamespaceShapeError(f"Invalid user_id: {validation_error}") if len(parts) == 2: return ResolvedNamespace( uri=f"viking://user/{user_id}", @@ -330,6 +382,7 @@ def _resolve_user_uri( canonical = f"viking://user/{user_id}" if suffix: canonical = f"{canonical}/{'/'.join(suffix)}" + _validate_peer_id_segments(parts) return ResolvedNamespace( uri=canonical, scope="user", @@ -337,6 +390,18 @@ def _resolve_user_uri( ) +def _is_current_user_relative_uri(parts: list[str], ctx: Optional[RequestContext]) -> bool: + if len(parts) < 2 or parts[0] != "user": + return False + if ctx is not None and parts[1] == ctx.user.user_id: + return False + return _is_user_relative_root_segment(parts[1]) + + +def _is_user_relative_root_segment(segment: str) -> bool: + return segment in _CONTENT_TYPES_BY_SCOPE["user"] or segment in _USER_RELATIVE_ROOT_SEGMENTS + + def _resolve_session_uri( parts: list[str], ctx: Optional[RequestContext], diff --git a/openviking/core/peer_id.py b/openviking/core/peer_id.py index 4246f06ca4..c1cdf1145a 100644 --- a/openviking/core/peer_id.py +++ b/openviking/core/peer_id.py @@ -4,24 +4,22 @@ from typing import Optional +from openviking.core.identifiers import normalize_identifier_part + def normalize_peer_id( peer_id: Optional[str], ) -> Optional[str]: """Normalize a peer_id value.""" - if peer_id == "": - peer_id = None - - normalized = peer_id - if normalized and ("/" in normalized or "\\" in normalized): - raise ValueError("peer_id must not contain path separators") - return normalized + try: + return normalize_identifier_part(peer_id, "peer_id") + except ValueError as exc: + raise ValueError(f"Invalid peer_id: {exc}") from exc def safe_peer_id(peer_id: Optional[str]) -> Optional[str]: """Return a usable peer_id, or None for empty/path-like values.""" - if not peer_id: - return None - if "/" in peer_id or "\\" in peer_id: + try: + return normalize_peer_id(peer_id) + except ValueError: return None - return peer_id diff --git a/openviking/core/retrieval_targets.py b/openviking/core/retrieval_targets.py index 12a624de4c..2c09877bcf 100644 --- a/openviking/core/retrieval_targets.py +++ b/openviking/core/retrieval_targets.py @@ -61,17 +61,16 @@ def default_target_directories( if not ctx or ctx.role == Role.ROOT: return [] - user_root = canonical_user_root(ctx) if context_type == ContextType.MEMORY: - return _default_memory_targets(ctx, peer_id) + return _default_user_scoped_targets(ctx, peer_id, "memories") if context_type == ContextType.RESOURCE: - return ["viking://resources"] + return _default_resource_targets(ctx, peer_id) if context_type == ContextType.SKILL: - return [f"{user_root}/skills"] + return _default_user_scoped_targets(ctx, peer_id, "skills") return [ - *_default_memory_targets(ctx, peer_id), - "viking://resources", - f"{user_root}/skills", + *_default_user_scoped_targets(ctx, peer_id, "memories"), + *_default_resource_targets(ctx, peer_id), + *_default_user_scoped_targets(ctx, peer_id, "skills"), ] @@ -107,32 +106,51 @@ def _target_directories_for_uri( peer_id: Optional[str], ) -> List[str]: if _is_current_user_root(target_uri, ctx): - return [*_default_memory_targets(ctx, peer_id), f"{canonical_user_root(ctx)}/skills"] + return _default_user_root_targets(ctx, peer_id) - peer_target = _resolve_peer_memory_target(target_uri, ctx=ctx, peer_id=peer_id) + peer_target = _resolve_peer_target(target_uri, ctx=ctx, peer_id=peer_id) if peer_target is not None: - return [peer_target] + return peer_target - if _is_default_user_memory_root(target_uri, ctx): - return _default_memory_targets(ctx, peer_id) + for segment in ("memories", "resources", "skills"): + if _is_default_user_content_root(target_uri, ctx, segment): + return _default_user_scoped_targets(ctx, peer_id, segment) return [target_uri] -def _default_memory_targets(ctx: RequestContext, peer_id: Optional[str]) -> List[str]: - user_root = canonical_user_root(ctx) - targets = [f"{user_root}/memories"] +def _default_user_root_targets(ctx: RequestContext, peer_id: Optional[str]) -> List[str]: + return [ + *_default_user_scoped_targets(ctx, peer_id, "memories"), + *_default_user_scoped_targets(ctx, peer_id, "resources"), + *_default_user_scoped_targets(ctx, peer_id, "skills"), + ] + + +def _default_resource_targets(ctx: RequestContext, peer_id: Optional[str]) -> List[str]: + return [ + "viking://resources", + *_default_user_scoped_targets(ctx, peer_id, "resources"), + ] + + +def _default_user_scoped_targets( + ctx: RequestContext, + peer_id: Optional[str], + segment: str, +) -> List[str]: + targets = [f"{canonical_user_root(ctx)}/{segment}"] if peer_id: - targets.append(f"{user_root}/peers/{peer_id}/memories") + targets.append(f"{canonical_user_root(ctx)}/peers/{peer_id}/{segment}") return targets -def _resolve_peer_memory_target( +def _resolve_peer_target( target_uri: str, *, ctx: RequestContext, peer_id: Optional[str], -) -> Optional[str]: +) -> Optional[List[str]]: parts = uri_parts(target_uri) user_root_parts = uri_parts(canonical_user_root(ctx)) if parts[: len(user_root_parts)] != user_root_parts: @@ -143,18 +161,22 @@ def _resolve_peer_memory_target( return None if len(suffix) == 1: - raise InvalidArgumentError("target_uri must not point at all peer memories.") + raise InvalidArgumentError("target_uri must not point at all peer contexts.") - target_peer_id = suffix[1] + target_peer_id = _normalize_peer_id(suffix[1]) if peer_id and target_peer_id != peer_id: raise InvalidArgumentError("target_uri peer does not match peer_id.") peer_root = f"{canonical_user_root(ctx)}/peers/{target_peer_id}" if len(suffix) == 2: - return f"{peer_root}/memories" - if suffix[2] != "memories": - raise InvalidArgumentError("Only peer memory targets are searchable.") - return target_uri + return [ + f"{peer_root}/memories", + f"{peer_root}/resources", + f"{peer_root}/skills", + ] + if suffix[2] not in {"memories", "resources", "skills"}: + raise InvalidArgumentError("Only peer memories, resources, and skills are searchable.") + return [target_uri] def _is_current_user_root(target_uri: str, ctx: RequestContext) -> bool: @@ -162,9 +184,9 @@ def _is_current_user_root(target_uri: str, ctx: RequestContext) -> bool: return normalized in {"viking://user", canonical_user_root(ctx).rstrip("/")} -def _is_default_user_memory_root(target_uri: str, ctx: RequestContext) -> bool: +def _is_default_user_content_root(target_uri: str, ctx: RequestContext, segment: str) -> bool: normalized = VikingURI.normalize(target_uri).rstrip("/") return normalized in { - "viking://user/memories", - f"{canonical_user_root(ctx).rstrip('/')}/memories", + f"viking://user/{segment}", + f"{canonical_user_root(ctx).rstrip('/')}/{segment}", } diff --git a/openviking/core/uri_validation.py b/openviking/core/uri_validation.py index 07497daa37..78b0500655 100644 --- a/openviking/core/uri_validation.py +++ b/openviking/core/uri_validation.py @@ -5,7 +5,14 @@ import re from collections.abc import Collection -from openviking_cli.exceptions import InvalidURIError +from openviking.core.namespace import ( + NamespaceShapeError, + canonicalize_uri, + classify_uri, + is_accessible, + uri_parts, +) +from openviking_cli.exceptions import InvalidURIError, PermissionDeniedError from openviking_cli.utils.uri import VikingURI _URI_SCHEME_RE = re.compile(r"^[A-Za-z][A-Za-z0-9+.-]*:") @@ -109,6 +116,61 @@ def validate_optional_viking_uri( ) +def validate_content_target_uri( + uri: str, + ctx, + *, + kind: str, + field_name: str = "uri", +) -> str: + """Validate and canonicalize add-resource/add-skill target URIs.""" + raw_uri = uri.strip() if isinstance(uri, str) else "" + if not raw_uri: + raise InvalidURIError(str(uri), f"{field_name} must not be empty") + + try: + canonical_uri = canonicalize_uri(raw_uri, ctx) + except (ValueError, NamespaceShapeError) as exc: + raise InvalidURIError(raw_uri, str(exc)) from exc + + if _matches_content_kind(canonical_uri, kind): + if is_accessible(canonical_uri, ctx): + return canonical_uri + raise PermissionDeniedError(f"Access denied for {canonical_uri}", resource=canonical_uri) + + raise InvalidURIError(raw_uri, f"{field_name} must target {kind} content") + + +def validate_optional_content_target_uri( + uri: str | None, + ctx, + *, + kind: str, + field_name: str = "uri", +) -> str: + if uri is None: + return "" + raw_uri = uri.strip() if isinstance(uri, str) else "" + if not raw_uri: + return "" + return validate_content_target_uri( + raw_uri, + ctx, + kind=kind, + field_name=field_name, + ) + + +def _matches_content_kind(uri: str, kind: str) -> bool: + if kind not in {"resource", "skill"}: + raise ValueError(f"Unsupported content target kind: {kind}") + parts = uri_parts(uri) + if kind == "resource" and parts[:1] == ["resources"]: + return True + classification = classify_uri(uri) + return classification.context_type == kind and classification.content_index is not None + + def validate_optional_viking_uris( uri: str | list[str] | None, *, diff --git a/openviking/parse/tree_builder.py b/openviking/parse/tree_builder.py index 18e33a6670..23aeebc5c6 100644 --- a/openviking/parse/tree_builder.py +++ b/openviking/parse/tree_builder.py @@ -25,6 +25,7 @@ from openviking.core.building_tree import BuildingTree from openviking.core.context import Context +from openviking.core.namespace import is_content_root_uri from openviking.parse.parsers.media.utils import get_media_base_uri, get_media_type from openviking.server.identity import RequestContext from openviking.storage.viking_fs import get_viking_fs @@ -95,9 +96,6 @@ async def resolve_target_uri( ) -> tuple[str, Optional[str]]: """Resolve the final target URI and optional unique-name candidate.""" - def is_resources_root(uri: Optional[str]) -> bool: - return (uri or "").rstrip("/") == "viking://resources" - final_doc_name = VikingURI.sanitize_segment(doc_name) if source_path and source_format == "repository": parsed_org_repo = parse_code_hosting_url(source_path) @@ -106,7 +104,7 @@ def is_resources_root(uri: Optional[str]) -> bool: auto_base_uri = self._get_base_uri(scope, source_path, source_format) base_uri = parent_uri or auto_base_uri - use_to_as_parent = is_resources_root(to_uri) + use_to_as_parent = bool(to_uri and is_content_root_uri(to_uri, ctx, kind="resource")) if to_uri and not use_to_as_parent: return to_uri, None @@ -115,26 +113,11 @@ def is_resources_root(uri: Optional[str]) -> bool: effective_parent_uri = effective_parent_uri.rstrip("/") if effective_parent_uri: viking_fs = get_viking_fs() - try: - parent_exists = await viking_fs.exists(effective_parent_uri, ctx=ctx) - if not parent_exists: - if create_parent: - logger.info( - f"[TreeBuilder] Parent URI does not exist, creating: {effective_parent_uri}" - ) - await viking_fs.mkdir(effective_parent_uri, exist_ok=True, ctx=ctx) - else: - raise FileNotFoundError( - f"Parent URI does not exist: {effective_parent_uri}. " - f"Use --parent-auto-create/-p to automatically create it." - ) - stat_result = await viking_fs.stat(effective_parent_uri, ctx=ctx) - except FileNotFoundError: - raise - except Exception as e: - raise FileNotFoundError(f"Parent URI does not exist: {effective_parent_uri}") from e - if not stat_result.get("isDir"): - raise ValueError(f"Parent URI is not a directory: {effective_parent_uri}") + await viking_fs.ensure_parent_exists( + effective_parent_uri, + ctx, + create_parent=create_parent, + ) base_uri = effective_parent_uri planned_uri = VikingURI(base_uri).join(final_doc_name).uri diff --git a/openviking/server/mcp_endpoint.py b/openviking/server/mcp_endpoint.py index d222606a98..96bd24a643 100644 --- a/openviking/server/mcp_endpoint.py +++ b/openviking/server/mcp_endpoint.py @@ -499,6 +499,7 @@ async def add_resource( result = await service.resources.add_resource( path=resolved.local_path, ctx=ctx, + to=to or None, reason=description, source_name=resolved.original_filename, wait=False, diff --git a/openviking/server/routers/resources.py b/openviking/server/routers/resources.py index 044976cfb9..821d61e7f0 100644 --- a/openviking/server/routers/resources.py +++ b/openviking/server/routers/resources.py @@ -7,7 +7,6 @@ from fastapi import APIRouter, Depends, File, Form, HTTPException, Query, Request, UploadFile from pydantic import BaseModel, ConfigDict, model_validator -from openviking.core.path_variables import resolve_path_variables from openviking.server.auth import get_request_context from openviking.server.dependencies import get_service from openviking.server.identity import RequestContext, Role @@ -98,6 +97,9 @@ class AddSkillRequest(BaseModel): data: Inline skill content or structured skill data. HTTP requests do not treat string values as host filesystem paths. temp_file_id: Temporary upload id returned by /api/v1/resources/temp_upload. + to: Exact target URI for the skill. + parent: Parent skills URI under which the skill name will be stored. + create_parent: Whether to automatically create parent when it does not exist. wait: Whether to wait for skill processing to complete. timeout: Timeout in seconds when wait=True. """ @@ -106,6 +108,9 @@ class AddSkillRequest(BaseModel): data: Any = None temp_file_id: Optional[str] = None + to: Optional[str] = None + parent: Optional[str] = None + create_parent: bool = False wait: bool = False timeout: Optional[float] = None source_metadata: Optional[Dict[str, Any]] = None @@ -193,8 +198,6 @@ async def add_resource( ): """Add resource to OpenViking.""" service = get_service() - if request.to and request.parent: - raise InvalidArgumentError("Cannot specify both 'to' and 'parent' at the same time.") path = request.path allow_local_path_resolution = False @@ -230,17 +233,13 @@ async def add_resource( if request.preserve_structure is not None: kwargs["preserve_structure"] = request.preserve_structure - # Resolve path variables before passing to service. - to = resolve_path_variables(request.to) if request.to else None - parent = resolve_path_variables(request.parent) if request.parent else None - async def _add() -> dict[str, Any]: try: result = await service.resources.add_resource( path=path, ctx=_ctx, - to=to, - parent=parent, + to=request.to, + parent=request.parent, reason=request.reason, instruction=request.instruction, wait=request.wait, @@ -277,6 +276,7 @@ async def add_skill( ): """Add skill to OpenViking.""" service = get_service() + data = request.data allow_local_path_resolution = False resolved = None @@ -308,6 +308,9 @@ async def _add() -> dict[str, Any]: result = await service.resources.add_skill( data=data, ctx=_ctx, + to=request.to, + parent=request.parent, + create_parent=request.create_parent, wait=request.wait, timeout=request.timeout, allow_local_path_resolution=allow_local_path_resolution, diff --git a/openviking/server/routers/skills.py b/openviking/server/routers/skills.py index 9f89ea9c4e..be65941a4d 100644 --- a/openviking/server/routers/skills.py +++ b/openviking/server/routers/skills.py @@ -13,8 +13,12 @@ from fastapi import Path as ApiPath from pydantic import BaseModel, ConfigDict, model_validator -from openviking.core.namespace import canonical_user_root +from openviking.core.namespace import ( + canonical_user_content_root, + is_content_root_uri, +) from openviking.core.skill_loader import SkillLoader +from openviking.core.uri_validation import validate_content_target_uri from openviking.privacy.service import UserPrivacyConfigVersion from openviking.server.auth import get_request_context from openviking.server.dependencies import get_service @@ -41,6 +45,7 @@ class UpdateSkillRequest(BaseModel): data: Any = None temp_file_id: Optional[str] = None + root_uri: Optional[str] = None wait: bool = False timeout: Optional[float] = None source_metadata: Optional[Dict[str, Any]] = None @@ -57,6 +62,7 @@ class FindSkillsRequest(BaseModel): model_config = ConfigDict(extra="forbid") query: str + root_uri: Optional[str] = None limit: int = 10 score_threshold: Optional[float] = None level: Optional[list[int]] = None @@ -73,15 +79,24 @@ class ValidateSkillRequest(BaseModel): def _agent_skills_root(ctx: RequestContext) -> str: - return f"{canonical_user_root(ctx)}/skills" + return canonical_user_content_root(ctx, "skill") + + +def _resolve_skills_root(ctx: RequestContext, root_uri: Optional[str]) -> str: + if not root_uri: + return _agent_skills_root(ctx) + resolved = validate_content_target_uri(root_uri, ctx, kind="skill", field_name="root_uri") + if not is_content_root_uri(resolved, ctx, kind="skill"): + raise InvalidArgumentError("root_uri must point to a skills root.") + return resolved.rstrip("/") def _validate_skill_name(skill_name: str) -> str: return validate_skill_name(skill_name) -def _skill_root_uri(ctx: RequestContext, skill_name: str) -> str: - return f"{_agent_skills_root(ctx)}/{_validate_skill_name(skill_name)}" +def _skill_root_uri(skills_root_uri: str, skill_name: str) -> str: + return f"{skills_root_uri.rstrip('/')}/{_validate_skill_name(skill_name)}" def _skill_md_uri(root_uri: str) -> str: @@ -289,8 +304,13 @@ def _skill_summary_from_hit(hit: Dict[str, Any]) -> Dict[str, Any]: return summary -async def _require_skill(service, ctx: RequestContext, skill_name: str) -> str: - root_uri = _skill_root_uri(ctx, skill_name) +async def _require_skill( + service, + ctx: RequestContext, + skill_name: str, + skills_root_uri: str, +) -> str: + root_uri = _skill_root_uri(skills_root_uri, skill_name) try: stat = await service.fs.stat(root_uri, ctx=ctx) except NotFoundError: @@ -411,11 +431,12 @@ async def _restore_skill_privacy( @router.get("") async def list_skills( node_limit: int = 1000, + root_uri: Optional[str] = None, _ctx: RequestContext = Depends(get_request_context), ): """List installed agent skills.""" service = get_service() - root_uri = _agent_skills_root(_ctx) + root_uri = _resolve_skills_root(_ctx, root_uri) try: entries = await service.fs.ls( root_uri, @@ -443,7 +464,7 @@ async def find_skills( ): """Find agent skills by semantic search.""" service = get_service() - root_uri = _agent_skills_root(_ctx) + root_uri = _resolve_skills_root(_ctx, request.root_uri) execution = await run_operation( operation="skills.find", telemetry=request.telemetry, @@ -491,6 +512,7 @@ async def get_skill( include_files: bool = True, include_source: bool = False, level: Optional[int] = None, + root_uri: Optional[str] = None, _ctx: RequestContext = Depends(get_request_context), ): """Show one installed agent skill.""" @@ -500,34 +522,36 @@ async def get_skill( details={"field": "level", "allowed": [0, 1, 2]}, ) service = get_service() - root_uri = await _require_skill(service, _ctx, skill_name) - abstract = await service.fs.abstract(root_uri, ctx=_ctx) - result = _skill_summary_from_meta(skill_name, root_uri, _parse_abstract_meta(abstract)) + skills_root_uri = _resolve_skills_root(_ctx, root_uri) + skill_root_uri = await _require_skill(service, _ctx, skill_name, skills_root_uri) + abstract = await service.fs.abstract(skill_root_uri, ctx=_ctx) + result = _skill_summary_from_meta(skill_name, skill_root_uri, _parse_abstract_meta(abstract)) if level is None or level == 0: result["abstract"] = abstract if level is None or level == 1: - result["overview"] = await service.fs.overview(root_uri, ctx=_ctx) + result["overview"] = await service.fs.overview(skill_root_uri, ctx=_ctx) if level == 2 or include_content is True or (level is None and include_content is not False): - result["content"] = await service.fs.read(_skill_md_uri(root_uri), ctx=_ctx) + result["content"] = await service.fs.read(_skill_md_uri(skill_root_uri), ctx=_ctx) if include_files: - entries = await _list_skill_files(service, _ctx, root_uri) + entries = await _list_skill_files(service, _ctx, skill_root_uri) result["files"] = [ { "name": entry.get("name") or _skill_name_from_uri(entry.get("uri", "")), "uri": entry.get("uri", ""), - "path": _relative_skill_path(root_uri, entry.get("uri", "")), + "path": _relative_skill_path(skill_root_uri, entry.get("uri", "")), "is_dir": entry.get("isDir", False), "kind": _skill_file_kind( - _relative_skill_path(root_uri, entry.get("uri", "")), + _relative_skill_path(skill_root_uri, entry.get("uri", "")), entry.get("isDir", False), ), } for entry in entries if isinstance(entry, dict) - and _relative_skill_path(root_uri, entry.get("uri", "")) != SOURCE_METADATA_FILENAME + and _relative_skill_path(skill_root_uri, entry.get("uri", "")) + != SOURCE_METADATA_FILENAME ] if include_source: - result["source"] = await read_skill_source_metadata(service, _ctx, root_uri) + result["source"] = await read_skill_source_metadata(service, _ctx, skill_root_uri) return Response(status="ok", result=result) @@ -536,11 +560,13 @@ async def update_skill( http_request: Request, request: UpdateSkillRequest, skill_name: str = ApiPath(..., description="Skill name"), + root_uri: Optional[str] = None, _ctx: RequestContext = Depends(get_request_context), ): """Replace an existing agent skill with new content.""" service = get_service() - root_uri = await _require_skill(service, _ctx, skill_name) + skills_root_uri = _resolve_skills_root(_ctx, root_uri) + skill_root_uri = await _require_skill(service, _ctx, skill_name, skills_root_uri) data = request.data allow_local_path_resolution = False @@ -569,7 +595,7 @@ async def update_skill( store = TempUploadStore.build(http_request.app.state.config) if resolved else None async def _update() -> Dict[str, Any]: - backup_uri = f"{_agent_skills_root(_ctx)}/.{skill_name}.update-backup-{uuid.uuid4().hex}" + backup_uri = f"{skills_root_uri}/.{skill_name}.update-backup-{uuid.uuid4().hex}" backup_created = False privacy_update_attempted = False previous_privacy = None @@ -593,11 +619,12 @@ async def _update() -> Dict[str, Any]: "actual": preparation.skill_dict.get("name"), }, ) - await service.fs.mv(root_uri, backup_uri, ctx=_ctx) + await service.fs.mv(skill_root_uri, backup_uri, ctx=_ctx) backup_created = True result = await service.resources.add_skill( data=preparation, ctx=_ctx, + to=skill_root_uri, wait=request.wait, timeout=request.timeout, allow_local_path_resolution=False, @@ -617,11 +644,11 @@ async def _update() -> Dict[str, Any]: except Exception: if backup_created: try: - await service.fs.rm(root_uri, ctx=_ctx, recursive=True) + await service.fs.rm(skill_root_uri, ctx=_ctx, recursive=True) except Exception: pass try: - await service.fs.mv(backup_uri, root_uri, ctx=_ctx) + await service.fs.mv(backup_uri, skill_root_uri, ctx=_ctx) except Exception: pass if privacy_update_attempted: @@ -660,17 +687,23 @@ async def _update() -> Dict[str, Any]: @router.delete("/{skill_name}") async def delete_skill( skill_name: str = ApiPath(..., description="Skill name"), + root_uri: Optional[str] = None, _ctx: RequestContext = Depends(get_request_context), ): """Remove one installed agent skill.""" service = get_service() - root_uri = await _require_skill(service, _ctx, skill_name) - result = await service.fs.rm(root_uri, ctx=_ctx, recursive=True) + skills_root_uri = _resolve_skills_root(_ctx, root_uri) + skill_root_uri = await _require_skill(service, _ctx, skill_name, skills_root_uri) + result = await service.fs.rm(skill_root_uri, ctx=_ctx, recursive=True) privacy_deleted = False privacy = service.privacy_configs if privacy is not None: privacy_deleted = await privacy.delete(_ctx, "skill", skill_name) - response_result: Dict[str, Any] = {"name": skill_name, "uri": root_uri, "root_uri": root_uri} + response_result: Dict[str, Any] = { + "name": skill_name, + "uri": skill_root_uri, + "root_uri": skill_root_uri, + } if isinstance(result, dict) and "estimated_deleted_count" in result: response_result["estimated_deleted_count"] = result["estimated_deleted_count"] response_result["privacy_deleted"] = privacy_deleted diff --git a/openviking/service/resource_service.py b/openviking/service/resource_service.py index e55b68f6cf..596e9aeca5 100644 --- a/openviking/service/resource_service.py +++ b/openviking/service/resource_service.py @@ -16,8 +16,8 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, List, Optional -from openviking.core.path_variables import resolve_path_variables -from openviking.core.uri_validation import validate_optional_viking_uri +from openviking.core.content_targets import ContentTargetSpec +from openviking.core.uri_validation import validate_optional_content_target_uri from openviking.server.identity import RequestContext from openviking.server.local_input_guard import ( is_remote_resource_source, @@ -150,19 +150,12 @@ async def enqueue_git_add_resource( """Start background ingestion for Git repositories while reserving the target URI.""" self._ensure_initialized() - if to: - to = resolve_path_variables(to) - if parent: - parent = resolve_path_variables(parent) - to = validate_optional_viking_uri( - to, - field_name="to", - allowed_scopes={"resources"}, - ) - parent = validate_optional_viking_uri( - parent, - field_name="parent", - allowed_scopes={"resources"}, + target = ContentTargetSpec.from_fields( + ctx=ctx, + kind="resource", + to=to, + parent=parent, + create_parent=bool(kwargs.get("create_parent", False)), ) from openviking.service.task_tracker import get_task_tracker @@ -180,11 +173,9 @@ async def enqueue_git_add_resource( root_uri, resource_lock = await self._plan_resource_target( path=path, ctx=ctx, - to=to, - parent=parent, + target=target, source_name=source_name, source_info=source_info, - create_parent=bool(kwargs.get("create_parent", False)), ) task_tracker = get_task_tracker() @@ -317,11 +308,9 @@ async def _plan_resource_target( *, path: str, ctx: RequestContext, - to: Optional[str], - parent: Optional[str], + target: ContentTargetSpec, source_name: Optional[str], source_info: _ResourceSourceInfo, - create_parent: bool, ) -> tuple[str, LockLease]: if not self._resource_processor or not self._viking_fs: raise NotInitializedError("ResourceProcessor") @@ -332,11 +321,11 @@ async def _plan_resource_target( ctx=ctx, doc_name=doc_name, scope="resources", - to_uri=to, - parent_uri=parent, + to_uri=target.to, + parent_uri=target.parent, source_path=source_path, source_format=source_info.source_format, - create_parent=create_parent, + create_parent=target.create_parent, ) if candidate_uri: return await self._resource_processor.reserve_unique_candidate( @@ -495,21 +484,12 @@ async def add_resource( telemetry.set("resource.flags.watch_enabled", watch_enabled) try: - # Resolve path variables before validation - if to: - to = resolve_path_variables(to) - if parent: - parent = resolve_path_variables(parent) - - to = validate_optional_viking_uri( - to, - field_name="to", - allowed_scopes={"resources"}, - ) - parent = validate_optional_viking_uri( - parent, - field_name="parent", - allowed_scopes={"resources"}, + target = ContentTargetSpec.from_fields( + ctx=ctx, + kind="resource", + to=to, + parent=parent, + create_parent=bool(kwargs.get("create_parent", False)), ) if enforce_public_remote_targets and is_remote_resource_source(path): path = require_remote_resource_source(path) @@ -523,8 +503,8 @@ async def add_resource( reason=reason, instruction=instruction, scope="resources", - to=to, - parent=parent, + to=target.to, + parent=target.parent, build_index=build_index, summarize=summarize, stage_callback=stage_callback, @@ -584,13 +564,14 @@ async def add_resource( if watch_manager and not skip_watch_management: with telemetry.measure("resource.watch"): if watch_interval > 0: - watch_to = to - parent_uri = parent + watch_to = target.to + parent_uri = target.parent if not watch_to: - watch_to = validate_optional_viking_uri( + watch_to = validate_optional_content_target_uri( result.get("root_uri"), + ctx, + kind="resource", field_name="root_uri", - allowed_scopes={"resources"}, ) parent_uri = None if not watch_to: @@ -825,6 +806,9 @@ async def add_skill( self, data: Any, ctx: RequestContext, + to: Optional[str] = None, + parent: Optional[str] = None, + create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, allow_local_path_resolution: bool = True, @@ -850,11 +834,19 @@ async def add_skill( request_wait_tracker.register_request(telemetry_id) try: + target = ContentTargetSpec.from_fields( + ctx=ctx, + kind="skill", + to=to, + parent=parent, + create_parent=create_parent, + ) if isinstance(data, SkillProcessingPreparation): result = await self._skill_processor.process_prepared_skill( data, viking_fs=self._viking_fs, ctx=ctx, + target=target, apply_privacy=apply_privacy, privacy_change_reason=privacy_change_reason, ) @@ -863,6 +855,7 @@ async def add_skill( data=data, viking_fs=self._viking_fs, ctx=ctx, + target=target, allow_local_path_resolution=allow_local_path_resolution, source_path_hint=source_path_hint, apply_privacy=apply_privacy, diff --git a/openviking/storage/ovpack/operations.py b/openviking/storage/ovpack/operations.py index 5edd2eaa30..4ce815cbdd 100644 --- a/openviking/storage/ovpack/operations.py +++ b/openviking/storage/ovpack/operations.py @@ -109,13 +109,6 @@ async def _root_exists(viking_fs, root_uri: str, ctx: RequestContext) -> bool: return False -async def _ensure_parent_exists(viking_fs, parent: str, ctx: RequestContext) -> None: - try: - await viking_fs.stat(parent, ctx=ctx) - except Exception: - await viking_fs.mkdir(parent, ctx=ctx) - - async def _remove_existing_root(viking_fs, root_uri: str, ctx: RequestContext) -> None: if not hasattr(viking_fs, "rm"): logger.warning(f"[ovpack] Cannot remove existing resource without rm(): {root_uri}") @@ -332,7 +325,7 @@ async def import_ovpack( vector_mode=vector_action_mode, ) if parent != "viking://": - await _ensure_parent_exists(viking_fs, parent, ctx) + await viking_fs.ensure_parent_exists(parent, ctx) for existing_root in existing_roots: logger.info(f"[ovpack] Overwriting existing resource at {existing_root}") diff --git a/openviking/storage/viking_fs.py b/openviking/storage/viking_fs.py index 72ed93a93a..6b0f483ba7 100644 --- a/openviking/storage/viking_fs.py +++ b/openviking/storage/viking_fs.py @@ -24,7 +24,7 @@ from pathlib import PurePath from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union -from openviking.core.namespace import canonicalize_uri +from openviking.core.namespace import canonicalize_uri, is_content_namespace_root_uri from openviking.core.namespace import ( is_accessible as namespace_is_accessible, ) @@ -288,6 +288,32 @@ def bind_request_context(self, ctx: RequestContext): finally: self._bound_ctx.reset(token) + async def ensure_parent_exists( + self, + parent: str, + ctx: RequestContext, + *, + create_parent: bool = True, + ) -> None: + try: + stat_result = await self.stat(parent, ctx=ctx) + except Exception as exc: + if not isinstance(exc, (FileNotFoundError, NotFoundError)) and not is_not_found_error( + exc + ): + raise + if create_parent or is_content_namespace_root_uri(parent, ctx): + await self.mkdir(parent, exist_ok=True, ctx=ctx) + stat_result = await self.stat(parent, ctx=ctx) + else: + raise FileNotFoundError( + f"Parent URI does not exist: {parent}. " + "Use --parent-auto-create/-p to automatically create it." + ) from exc + + if not stat_result.get("isDir"): + raise InvalidArgumentError(f"Parent URI is not a directory: {parent}") + @staticmethod def _normalize_uri(uri: str) -> str: """Normalize short-format URIs to the canonical viking:// form.""" @@ -1669,9 +1695,6 @@ def _shorten_component(component: str, max_bytes: int = 255) -> str: prefix = prefix[:-1] return f"{prefix}_{hash_suffix}" - _USER_STRUCTURE_DIRS = {"memories"} - _AGENT_STRUCTURE_DIRS = {"memories", "skills", "instructions", "workspaces"} - def _uri_to_path(self, uri: str, ctx: Optional[RequestContext] = None) -> str: """Map virtual URI to account-isolated AGFS path. @@ -1738,35 +1761,6 @@ def _is_legacy_temp_uri_parts(self, parts: List[str]) -> bool: return True return not self._looks_like_legacy_temp_leaf(parts[2]) - def _extract_space_from_uri(self, uri: str) -> Optional[str]: - """Extract space segment from URI if present. - - URIs are WYSIWYG: viking://{scope}/{space}/... - For user, the second segment is space unless it's a known structure dir. - For session, the second segment is always space (when 3+ parts). - Legacy temp URIs keep the historical shape viking://temp/ and therefore - intentionally have no space segment. - """ - _, parts = self._normalized_uri_parts(uri) - if len(parts) < 2: - return None - scope = parts[0] - second = parts[1] - # Treat scope-root metadata files as not having a tenant space segment. - if len(parts) == 2 and second in {".abstract.md", ".overview.md"}: - return None - if self._is_legacy_temp_uri_parts(parts): - return None - if scope == "upload": - return None - if scope == "user" and second not in self._USER_STRUCTURE_DIRS: - return second - if scope == "agent" and second not in self._AGENT_STRUCTURE_DIRS: - return second - if scope == "session" and len(parts) >= 2: - return second - return None - def _is_accessible(self, uri: str, ctx: RequestContext) -> bool: """Check whether a URI is visible/accessible under current request context.""" normalized_uri, parts = self._normalized_uri_parts(uri) diff --git a/openviking/sync_client.py b/openviking/sync_client.py index 6ecc91726a..e7359335f8 100644 --- a/openviking/sync_client.py +++ b/openviking/sync_client.py @@ -210,13 +210,24 @@ def add_resource( def add_skill( self, data: Any, + to: Optional[str] = None, + parent: Optional[str] = None, + create_parent: bool = False, wait: bool = False, timeout: float = None, telemetry: TelemetryRequest = False, ) -> Dict[str, Any]: """Add skill to OpenViking.""" return run_async( - self._async_client.add_skill(data, wait=wait, timeout=timeout, telemetry=telemetry) + self._async_client.add_skill( + data, + to=to, + parent=parent, + create_parent=create_parent, + wait=wait, + timeout=timeout, + telemetry=telemetry, + ) ) def search( diff --git a/openviking/utils/skill_processor.py b/openviking/utils/skill_processor.py index dd7f1fcab2..c95b129ee5 100644 --- a/openviking/utils/skill_processor.py +++ b/openviking/utils/skill_processor.py @@ -17,9 +17,14 @@ import yaml +from openviking.core.content_targets import ContentTargetSpec from openviking.core.context import Context, ContextType, Vectorize from openviking.core.mcp_converter import is_mcp_format, mcp_to_skill -from openviking.core.namespace import canonical_user_root, user_space_fragment +from openviking.core.namespace import ( + canonical_user_content_root, + uri_leaf_name, + user_space_fragment, +) from openviking.core.skill_loader import SkillLoader from openviking.privacy import ( UserPrivacyConfigService, @@ -36,6 +41,7 @@ from openviking_cli.exceptions import InvalidArgumentError from openviking_cli.utils import get_logger from openviking_cli.utils.config import get_openviking_config +from openviking_cli.utils.uri import VikingURI logger = get_logger(__name__) @@ -113,6 +119,7 @@ async def process_skill( data: Any, viking_fs: VikingFS, ctx: RequestContext, + target: Optional[ContentTargetSpec] = None, allow_local_path_resolution: bool = True, source_path_hint: Optional[str] = None, apply_privacy: bool = True, @@ -148,6 +155,7 @@ async def process_skill( preparation, viking_fs=viking_fs, ctx=ctx, + target=target, apply_privacy=apply_privacy, privacy_change_reason=privacy_change_reason, ) @@ -158,6 +166,7 @@ async def process_prepared_skill( viking_fs: VikingFS, ctx: RequestContext, *, + target: Optional[ContentTargetSpec] = None, apply_privacy: bool = True, privacy_change_reason: str = "auto-extracted from add_skill", ) -> Dict[str, Any]: @@ -178,9 +187,14 @@ async def process_prepared_skill( ) skill_abstract = self._build_skill_abstract(skill_dict) - skill_root_uri = f"{canonical_user_root(ctx)}/skills" + skill_dir_uri, skill_root_uri = await self._resolve_skill_target_uri( + viking_fs=viking_fs, + ctx=ctx, + skill_name=skill_dict["name"], + target=target, + ) context = Context( - uri=f"{skill_root_uri}/{skill_dict['name']}", + uri=skill_dir_uri, parent_uri=skill_root_uri, is_leaf=False, abstract=skill_abstract, @@ -205,8 +219,6 @@ async def process_prepared_skill( round((time.perf_counter() - overview_start) * 1000, 3), ) - skill_dir_uri = context.uri - write_start = time.perf_counter() await self._write_skill_content( viking_fs=viking_fs, @@ -274,6 +286,40 @@ async def prepare_skill_processing( shutil.rmtree(cleanup_path, ignore_errors=True) raise + async def _resolve_skill_target_uri( + self, + *, + viking_fs: VikingFS, + ctx: RequestContext, + skill_name: str, + target: Optional[ContentTargetSpec], + ) -> tuple[str, str]: + target = target or ContentTargetSpec() + if target.to: + target_uri = target.to.rstrip("/") + target_name = uri_leaf_name(target_uri) + if target_name != skill_name: + raise InvalidArgumentError( + f"Skill target URI name mismatch: target name is '{target_name}', " + f"content name is '{skill_name}'", + details={"expected": skill_name, "actual": target_name}, + ) + parent_uri_obj = VikingURI(target_uri).parent + if parent_uri_obj is None: + raise InvalidArgumentError( + f"Skill target URI must be under a skills root: {target_uri}" + ) + return target_uri, parent_uri_obj.uri + + skill_root_uri = (target.parent or canonical_user_content_root(ctx, "skill")).rstrip("/") + if target.parent: + await viking_fs.ensure_parent_exists( + skill_root_uri, + ctx, + create_parent=target.create_parent, + ) + return f"{skill_root_uri}/{skill_name}", skill_root_uri + def _parse_skill( self, data: Any, diff --git a/openviking_cli/client/base.py b/openviking_cli/client/base.py index 066f12b6d1..490587f1a5 100644 --- a/openviking_cli/client/base.py +++ b/openviking_cli/client/base.py @@ -51,6 +51,9 @@ async def add_resource( async def add_skill( self, data: Any, + to: Optional[str] = None, + parent: Optional[str] = None, + create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, telemetry: TelemetryRequest = False, diff --git a/openviking_cli/client/http.py b/openviking_cli/client/http.py index cb84fd31e0..3e3ab04c8c 100644 --- a/openviking_cli/client/http.py +++ b/openviking_cli/client/http.py @@ -477,6 +477,9 @@ async def batch_add_messages( async def add_skill( self, data: Any, + to: Optional[str] = None, + parent: Optional[str] = None, + create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, telemetry: TelemetryRequest = False, @@ -484,6 +487,9 @@ async def add_skill( """Add skill to OpenViking.""" telemetry = self._validate_telemetry(telemetry) request_data = { + "to": to, + "parent": parent, + "create_parent": create_parent, "wait": wait, "timeout": timeout, } diff --git a/openviking_cli/client/sync_http.py b/openviking_cli/client/sync_http.py index a624896f83..064dcce0b0 100644 --- a/openviking_cli/client/sync_http.py +++ b/openviking_cli/client/sync_http.py @@ -260,13 +260,24 @@ def add_resource( def add_skill( self, data: Any, + to: Optional[str] = None, + parent: Optional[str] = None, + create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, telemetry: TelemetryRequest = False, ) -> Dict[str, Any]: """Add skill to OpenViking.""" return run_async( - self._async_client.add_skill(data, wait=wait, timeout=timeout, telemetry=telemetry) + self._async_client.add_skill( + data, + to=to, + parent=parent, + create_parent=create_parent, + wait=wait, + timeout=timeout, + telemetry=telemetry, + ) ) def wait_processed(self, timeout: Optional[float] = None) -> Dict[str, Any]: diff --git a/openviking_cli/session/user_id.py b/openviking_cli/session/user_id.py index 07c2c55760..11c3d42d8e 100644 --- a/openviking_cli/session/user_id.py +++ b/openviking_cli/session/user_id.py @@ -1,41 +1,20 @@ -import re - +from openviking.core.identifiers import ( + normalize_identifier_part, + validate_account_id, + validate_identifier_part, + validate_user_id, +) from openviking_cli.utils import get_logger logger = get_logger(__name__) -# Validation pattern reused across different modules -# Note: hyphen (-) must be at the end or escaped to avoid being interpreted as a range -_VALIDATION_PATTERN = re.compile(r"^[a-zA-Z0-9_.@-]+$") - - -def validate_identifier_part(part: str, part_name: str) -> str | None: - """Validate a single part of an identifier (account_id or user_id). - - Returns an error message if invalid, None if valid. - """ - if not part: - return f"{part_name} is empty" - if not _VALIDATION_PATTERN.match(part): - return f"{part_name} must be alpha_numeric string." - if part.count("@") > 1: - return f"{part_name} must have at most one @." - return None - - -def validate_account_id(account_id: str) -> str | None: - """Validate an account_id. Returns an error message if invalid, None if valid.""" - verr = validate_identifier_part(account_id, "account_id") - if verr: - return verr - if account_id.startswith("_"): - return "account_id cannot start with underscore _." - return None - - -def validate_user_id(user_id: str) -> str | None: - """Validate a user_id. Returns an error message if invalid, None if valid.""" - return validate_identifier_part(user_id, "user_id") +__all__ = [ + "UserIdentifier", + "normalize_identifier_part", + "validate_account_id", + "validate_identifier_part", + "validate_user_id", +] class UserIdentifier(object): @@ -79,9 +58,6 @@ def user_space_name(self) -> str: def memory_space_uri(self) -> str: return f"viking://user/{self.user_space_name()}/memories" - def work_space_uri(self) -> str: - return f"viking://user/{self.user_space_name()}/workspaces" - def to_dict(self): return { "account_id": self._account_id, diff --git a/tests/cli/test_user_identifier.py b/tests/cli/test_user_identifier.py index 73464061cd..532126d406 100644 --- a/tests/cli/test_user_identifier.py +++ b/tests/cli/test_user_identifier.py @@ -1,5 +1,7 @@ """Tests for account/user-only UserIdentifier.""" +import pytest + from openviking_cli.session.user_id import UserIdentifier @@ -22,3 +24,8 @@ def test_memory_space_uri_uses_user_space(self): assert u.user_space_name() == "user1" assert u.memory_space_uri() == "viking://user/user1/memories" assert u.to_dict() == {"account_id": "acct", "user_id": "user1"} + + @pytest.mark.parametrize("user_id", [".", "..", "team:alice"]) + def test_user_id_rejects_unsafe_path_segments(self, user_id): + with pytest.raises(ValueError): + UserIdentifier("acct", user_id) diff --git a/tests/client/test_http_client_config.py b/tests/client/test_http_client_config.py index ede7dfc951..2089ece98c 100644 --- a/tests/client/test_http_client_config.py +++ b/tests/client/test_http_client_config.py @@ -311,7 +311,7 @@ async def test_async_http_client_find_sends_peer_id(tmp_path, monkeypatch): await client.find( "invoice", target_uri="viking://user/memories", - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) assert http.calls == [ @@ -324,7 +324,7 @@ async def test_async_http_client_find_sends_peer_id(tmp_path, monkeypatch): "score_threshold": None, "filter": None, "telemetry": False, - "peer_id": "web:visitor:alice", + "peer_id": "web-visitor-alice", }, ) ] @@ -344,7 +344,7 @@ async def test_async_http_client_search_sends_peer_id(tmp_path, monkeypatch): "invoice", target_uri="viking://user/memories", session_id="session-1", - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) assert http.calls == [ @@ -358,7 +358,7 @@ async def test_async_http_client_search_sends_peer_id(tmp_path, monkeypatch): "score_threshold": None, "filter": None, "telemetry": False, - "peer_id": "web:visitor:alice", + "peer_id": "web-visitor-alice", }, ) ] diff --git a/tests/server/test_api_resources.py b/tests/server/test_api_resources.py index c3c2ce516e..6c8bcf19c0 100644 --- a/tests/server/test_api_resources.py +++ b/tests/server/test_api_resources.py @@ -328,6 +328,52 @@ async def test_add_resource_with_resources_root_to_uses_child_uri( assert body["result"]["root_uri"] == "viking://resources/tt_b" +async def test_add_resource_with_user_resources_short_parent_initializes_root( + client: httpx.AsyncClient, + upload_temp_dir, +): + archive_path = upload_temp_dir / "user_short_docs.zip" + with zipfile.ZipFile(archive_path, "w") as zf: + zf.writestr("user_short_docs/readme.md", "# hello\n") + + resp = await client.post( + "/api/v1/resources", + json={ + "temp_file_id": archive_path.name, + "parent": "viking://user/resources", + "reason": "test user resource short parent import", + "wait": True, + }, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["status"] == "ok" + assert body["result"]["root_uri"] == "viking://user/default/resources/user_short_docs" + + +async def test_add_resource_with_peer_resources_root_to_uses_child_uri( + client: httpx.AsyncClient, + upload_temp_dir, +): + archive_path = upload_temp_dir / "peer_docs.zip" + with zipfile.ZipFile(archive_path, "w") as zf: + zf.writestr("peer_docs/readme.md", "# hello\n") + + resp = await client.post( + "/api/v1/resources", + json={ + "temp_file_id": archive_path.name, + "to": "viking://user/default/peers/alice/resources", + "reason": "test peer resource root import", + "wait": True, + }, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["status"] == "ok" + assert body["result"]["root_uri"] == "viking://user/default/peers/alice/resources/peer_docs" + + async def test_add_resource_with_resources_root_to_trailing_slash_uses_child_uri( client: httpx.AsyncClient, upload_temp_dir, diff --git a/tests/server/test_api_skills.py b/tests/server/test_api_skills.py index 83fac085d7..7c0067cfd8 100644 --- a/tests/server/test_api_skills.py +++ b/tests/server/test_api_skills.py @@ -169,6 +169,71 @@ async def test_skills_api_list_show_find_and_delete(client): assert missing_privacy_response.status_code == 404 +async def test_skills_api_manages_peer_skills_with_root_uri(client): + peer_root_uri = "viking://user/default/peers/alice/skills" + add_response = await client.post( + "/api/v1/skills", + json={ + "data": _skill_md("peer-skill", "Peer skill"), + "to": f"{peer_root_uri}/peer-skill", + "wait": True, + }, + ) + assert add_response.status_code == 200, add_response.text + assert add_response.json()["result"]["root_uri"] == f"{peer_root_uri}/peer-skill" + + default_list_response = await client.get("/api/v1/skills") + assert default_list_response.status_code == 200, default_list_response.text + assert all( + skill["name"] != "peer-skill" + for skill in default_list_response.json()["result"]["skills"] + ) + + peer_list_response = await client.get( + "/api/v1/skills", + params={"root_uri": peer_root_uri}, + ) + assert peer_list_response.status_code == 200, peer_list_response.text + assert any( + skill["name"] == "peer-skill" + for skill in peer_list_response.json()["result"]["skills"] + ) + + show_response = await client.get( + "/api/v1/skills/peer-skill", + params={"root_uri": peer_root_uri, "level": 2}, + ) + assert show_response.status_code == 200, show_response.text + assert show_response.json()["result"]["root_uri"] == f"{peer_root_uri}/peer-skill" + + find_response = await client.post( + "/api/v1/skills/find", + json={"query": "peer", "root_uri": peer_root_uri, "limit": 5}, + ) + assert find_response.status_code == 200, find_response.text + assert find_response.json()["result"]["root_uri"] == peer_root_uri + + delete_response = await client.delete( + "/api/v1/skills/peer-skill", + params={"root_uri": peer_root_uri}, + ) + assert delete_response.status_code == 200, delete_response.text + assert delete_response.json()["result"]["root_uri"] == f"{peer_root_uri}/peer-skill" + + +async def test_skills_api_rejects_skill_target_under_resources(client): + response = await client.post( + "/api/v1/skills", + json={ + "data": _skill_md("wrong-root-skill", "Wrong root skill"), + "parent": "viking://user/default/peers/alice/resources", + "wait": True, + }, + ) + assert response.status_code == 400, response.text + assert response.json()["error"]["code"] == "INVALID_URI" + + async def test_skills_api_update_requires_matching_name(client): await _add_skill(client, "update-skill", "Original description") diff --git a/tests/server/test_peer_id_compat.py b/tests/server/test_peer_id_compat.py index 043ddae0d4..9887590bf0 100644 --- a/tests/server/test_peer_id_compat.py +++ b/tests/server/test_peer_id_compat.py @@ -14,7 +14,12 @@ def test_normalize_peer_id_accepts_peer_id(): - assert normalize_peer_id("web:visitor:alice") == "web:visitor:alice" + assert normalize_peer_id("web-visitor-alice") == "web-visitor-alice" + + +def test_normalize_peer_id_rejects_colon(): + with pytest.raises(ValueError, match="Invalid peer_id"): + normalize_peer_id("web:visitor:alice") def _target_dirs(target_uri="", peer_id=None): @@ -22,101 +27,117 @@ def _target_dirs(target_uri="", peer_id=None): return resolve_retrieval_targets(target_uri, ctx, peer_id).target_directories -def test_default_search_targets_user_memory_without_all_peer_memories(): +def test_default_search_targets_user_content_without_all_peer_content(): targets = _target_dirs() assert targets == [ "viking://user/support_bot/memories", "viking://resources", + "viking://user/support_bot/resources", "viking://user/support_bot/skills", ] -def test_default_peer_search_targets_self_and_requested_peer_memories(): - targets = _target_dirs(peer_id="web:visitor:alice") +def test_default_peer_search_targets_self_and_requested_peer_content(): + targets = _target_dirs(peer_id="web-visitor-alice") assert targets == [ "viking://user/support_bot/memories", - "viking://user/support_bot/peers/web:visitor:alice/memories", + "viking://user/support_bot/peers/web-visitor-alice/memories", "viking://resources", + "viking://user/support_bot/resources", + "viking://user/support_bot/peers/web-visitor-alice/resources", "viking://user/support_bot/skills", + "viking://user/support_bot/peers/web-visitor-alice/skills", ] def test_peer_search_keeps_explicit_target_uri(): - targets = _target_dirs("viking://resources/docs", peer_id="web:visitor:alice") + targets = _target_dirs("viking://resources/docs", peer_id="web-visitor-alice") assert targets == ["viking://resources/docs"] def test_peer_search_expands_default_user_memory_target(): - targets = _target_dirs("viking://user/memories", peer_id="web:visitor:alice") + targets = _target_dirs("viking://user/memories", peer_id="web-visitor-alice") assert targets == [ "viking://user/support_bot/memories", - "viking://user/support_bot/peers/web:visitor:alice/memories", + "viking://user/support_bot/peers/web-visitor-alice/memories", ] -def test_peer_search_explicit_peer_root_targets_that_peer_memory(): - targets = _target_dirs("viking://user/support_bot/peers/web:visitor:alice") +def test_peer_search_explicit_peer_root_targets_that_peer_content(): + targets = _target_dirs("viking://user/support_bot/peers/web-visitor-alice") - assert targets == ["viking://user/support_bot/peers/web:visitor:alice/memories"] + assert targets == [ + "viking://user/support_bot/peers/web-visitor-alice/memories", + "viking://user/support_bot/peers/web-visitor-alice/resources", + "viking://user/support_bot/peers/web-visitor-alice/skills", + ] def test_peer_search_explicit_peer_memory_matches_peer_id(): targets = _target_dirs( - "viking://user/support_bot/peers/web:visitor:alice/memories", - peer_id="web:visitor:alice", + "viking://user/support_bot/peers/web-visitor-alice/memories", + peer_id="web-visitor-alice", ) - assert targets == ["viking://user/support_bot/peers/web:visitor:alice/memories"] + assert targets == ["viking://user/support_bot/peers/web-visitor-alice/memories"] def test_peer_search_explicit_peer_target_rejects_mismatched_peer_id(): with pytest.raises(InvalidArgumentError, match="target_uri peer does not match peer_id"): _target_dirs( - "viking://user/support_bot/peers/web:visitor:alice/memories", - peer_id="web:visitor:bob", + "viking://user/support_bot/peers/web-visitor-alice/memories", + peer_id="web-visitor-bob", ) +def test_peer_search_explicit_peer_target_rejects_invalid_peer_id(): + with pytest.raises(InvalidArgumentError, match="Invalid peer_id"): + _target_dirs("viking://user/support_bot/peers/web:visitor:alice/memories") + + def test_peer_search_rejects_all_peers_target(): - with pytest.raises(InvalidArgumentError, match="all peer memories"): + with pytest.raises(InvalidArgumentError, match="all peer contexts"): _target_dirs("viking://user/support_bot/peers") -def test_peer_search_user_root_targets_user_memory_and_skills_only(): - targets = _target_dirs("viking://user", peer_id="web:visitor:alice") +def test_peer_search_user_root_targets_user_content_and_requested_peer_content(): + targets = _target_dirs("viking://user", peer_id="web-visitor-alice") assert targets == [ "viking://user/support_bot/memories", - "viking://user/support_bot/peers/web:visitor:alice/memories", + "viking://user/support_bot/peers/web-visitor-alice/memories", + "viking://user/support_bot/resources", + "viking://user/support_bot/peers/web-visitor-alice/resources", "viking://user/support_bot/skills", + "viking://user/support_bot/peers/web-visitor-alice/skills", ] def test_peer_search_expands_canonical_user_memory_target(): targets = _target_dirs( "viking://user/support_bot/memories", - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) assert targets == [ "viking://user/support_bot/memories", - "viking://user/support_bot/peers/web:visitor:alice/memories", + "viking://user/support_bot/peers/web-visitor-alice/memories", ] def test_peer_search_list_expands_only_default_memory_targets(): targets = _target_dirs( ["viking://user/memories", "viking://resources/docs"], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) assert targets == [ "viking://user/support_bot/memories", - "viking://user/support_bot/peers/web:visitor:alice/memories", + "viking://user/support_bot/peers/web-visitor-alice/memories", "viking://resources/docs", ] diff --git a/tests/session/memory/test_memory_isolation_handler.py b/tests/session/memory/test_memory_isolation_handler.py index 8e7ebcf35e..ebe8ba518c 100644 --- a/tests/session/memory/test_memory_isolation_handler.py +++ b/tests/session/memory/test_memory_isolation_handler.py @@ -69,8 +69,8 @@ def test_message_peer_ids_do_not_expand_self_extraction_scope(self): """Message peer_id should not make self extraction read or write peer memory.""" ctx = create_ctx() messages = [ - create_message("user", peer_id="web:visitor:alice"), - create_message("user", peer_id="web:visitor:bob"), + create_message("user", peer_id="web-visitor-alice"), + create_message("user", peer_id="web-visitor-bob"), ] extract_ctx = create_mock_extract_context(messages) handler = MemoryIsolationHandler(ctx, extract_ctx) @@ -189,16 +189,16 @@ def test_fill_identity_fields_with_ranges_keeps_ctx_user_only(self): def test_fill_identity_fields_normalizes_explicit_peer_id(self): ctx = create_ctx() - messages = [create_message("user", peer_id="web:visitor:alice")] + messages = [create_message("user", peer_id="web-visitor-alice")] extract_ctx = create_mock_extract_context(messages) handler = MemoryIsolationHandler(ctx, extract_ctx) role_scope = handler.get_read_scope() - item_dict = {"peer_id": "web:visitor:alice"} + item_dict = {"peer_id": "web-visitor-alice"} handler.fill_identity_fields(item_dict, role_scope) assert item_dict["user_id"] == "user_a" - assert item_dict["peer_id"] == "web:visitor:alice" + assert item_dict["peer_id"] == "web-visitor-alice" class TestPrepareMessages: @@ -209,13 +209,13 @@ def test_prepare_messages_keeps_peer_metadata(self): messages = [ create_message("user", "Hello"), create_message("assistant", "Hi"), - create_message("user", "Hey", peer_id="web:visitor:alice"), + create_message("user", "Hey", peer_id="web-visitor-alice"), ] extract_ctx = create_mock_extract_context(messages) handler = MemoryIsolationHandler(ctx, extract_ctx) handler.prepare_messages() - assert messages[2].peer_id == "web:visitor:alice" + assert messages[2].peer_id == "web-visitor-alice" def test_get_read_scope_uses_ctx_user(self): ctx = create_ctx(user_id="login_user") @@ -233,7 +233,7 @@ def test_get_read_scope_uses_ctx_user(self): def test_get_read_scope_ignores_message_peer_id_without_target(self): ctx = create_ctx(user_id="login_user") messages = [ - create_message("user", "Hello", peer_id="web:visitor:alice"), + create_message("user", "Hello", peer_id="web-visitor-alice"), create_message("assistant", "Hi"), ] extract_ctx = create_mock_extract_context(messages) @@ -247,19 +247,19 @@ def test_get_read_scope_ignores_message_peer_id_without_target(self): def test_get_read_scope_includes_allowed_peer_ids_when_enabled(self): ctx = create_ctx(user_id="login_user") messages = [ - create_message("user", "Hello", peer_id="web:visitor:alice"), + create_message("user", "Hello", peer_id="web-visitor-alice"), ] extract_ctx = create_mock_extract_context(messages) handler = MemoryIsolationHandler( ctx, extract_ctx, - allowed_peer_ids={"web:visitor:alice"}, + allowed_peer_ids={"web-visitor-alice"}, ) handler.prepare_messages() scope = handler.get_read_scope() assert scope.user_ids == ["login_user"] - assert scope.peer_ids == ["web:visitor:alice"] + assert scope.peer_ids == ["web-visitor-alice"] class TestCalculateMemoryUris: @@ -336,12 +336,12 @@ def test_calculate_memory_uris_routes_explicit_peer_memory(self, mock_generate_u ctx = create_ctx( user_id="support_bot", ) - messages = [create_message("user", peer_id="web:visitor:alice")] + messages = [create_message("user", peer_id="web-visitor-alice")] extract_ctx = create_mock_extract_context(messages) handler = MemoryIsolationHandler( ctx, extract_ctx, - allowed_peer_ids={"web:visitor:alice"}, + allowed_peer_ids={"web-visitor-alice"}, ) from openviking.session.memory.dataclass import MemoryTypeSchema, ResolvedOperation @@ -355,7 +355,7 @@ def test_calculate_memory_uris_routes_explicit_peer_memory(self, mock_generate_u old_memory_file_content=None, memory_fields={ "user_id": "alice", - "peer_id": "web:visitor:alice", + "peer_id": "web-visitor-alice", }, memory_type="preferences", uris=[], @@ -363,9 +363,9 @@ def test_calculate_memory_uris_routes_explicit_peer_memory(self, mock_generate_u uris = handler.calculate_memory_uris(schema, operation, extract_ctx) - assert uris == ["viking://user/support_bot/peers/web:visitor:alice/memories/preferences"] + assert uris == ["viking://user/support_bot/peers/web-visitor-alice/memories/preferences"] assert operation.memory_fields["user_id"] == "support_bot" - assert operation.memory_fields["peer_id"] == "web:visitor:alice" + assert operation.memory_fields["peer_id"] == "web-visitor-alice" @patch("openviking.session.memory.memory_isolation_handler.generate_uri") def test_calculate_memory_uris_routes_ranges_to_self_and_peer(self, mock_generate_uri): @@ -376,7 +376,7 @@ def test_calculate_memory_uris_routes_ranges_to_self_and_peer(self, mock_generat ctx = create_ctx(user_id="support_bot") messages = [ create_message("user", "self event"), - create_message("user", "peer event", peer_id="web:visitor:alice"), + create_message("user", "peer event", peer_id="web-visitor-alice"), ] extract_ctx = create_mock_extract_context(messages) mock_range = MagicMock() @@ -386,7 +386,7 @@ def test_calculate_memory_uris_routes_ranges_to_self_and_peer(self, mock_generat ctx, extract_ctx, allow_self=True, - allowed_peer_ids={"web:visitor:alice"}, + allowed_peer_ids={"web-visitor-alice"}, ) from openviking.session.memory.dataclass import MemoryTypeSchema, ResolvedOperation @@ -407,7 +407,7 @@ def test_calculate_memory_uris_routes_ranges_to_self_and_peer(self, mock_generat assert set(uris) == { "viking://user/support_bot/memories/events/demo", - "viking://user/support_bot/peers/web:visitor:alice/memories/events/demo", + "viking://user/support_bot/peers/web-visitor-alice/memories/events/demo", } assert operation.memory_fields["user_id"] == "support_bot" assert "peer_id" not in operation.memory_fields @@ -415,12 +415,12 @@ def test_calculate_memory_uris_routes_ranges_to_self_and_peer(self, mock_generat @patch("openviking.session.memory.memory_isolation_handler.generate_uri") def test_calculate_memory_uris_rejects_unallowed_peer_id(self, mock_generate_uri): ctx = create_ctx(user_id="support_bot") - messages = [create_message("user", peer_id="web:visitor:alice")] + messages = [create_message("user", peer_id="web-visitor-alice")] extract_ctx = create_mock_extract_context(messages) handler = MemoryIsolationHandler( ctx, extract_ctx, - allowed_peer_ids={"web:visitor:bob"}, + allowed_peer_ids={"web-visitor-bob"}, ) from openviking.session.memory.dataclass import MemoryTypeSchema, ResolvedOperation @@ -432,7 +432,7 @@ def test_calculate_memory_uris_rejects_unallowed_peer_id(self, mock_generate_uri ) operation = ResolvedOperation( old_memory_file_content=None, - memory_fields={"peer_id": "web:visitor:alice"}, + memory_fields={"peer_id": "web-visitor-alice"}, memory_type="preferences", uris=[], ) @@ -453,15 +453,15 @@ def test_calculate_memory_uris_missing_peer_id_prefers_self_when_self_user_messa ctx = create_ctx(user_id="support_bot") messages = [ create_message("user", "self turn"), - create_message("assistant", "ack", peer_id="web:visitor:alice"), - create_message("user", "peer turn", peer_id="web:visitor:alice"), + create_message("assistant", "ack", peer_id="web-visitor-alice"), + create_message("user", "peer turn", peer_id="web-visitor-alice"), ] extract_ctx = create_mock_extract_context(messages) handler = MemoryIsolationHandler( ctx, extract_ctx, allow_self=True, - allowed_peer_ids={"web:visitor:alice"}, + allowed_peer_ids={"web-visitor-alice"}, ) from openviking.session.memory.dataclass import MemoryTypeSchema, ResolvedOperation @@ -494,16 +494,16 @@ def test_calculate_memory_uris_missing_peer_id_falls_back_to_first_peer_when_sel ctx = create_ctx(user_id="support_bot") messages = [ - create_message("user", "peer turn one", peer_id="web:visitor:bob"), - create_message("assistant", "ack", peer_id="web:visitor:bob"), - create_message("user", "peer turn two", peer_id="web:visitor:alice"), + create_message("user", "peer turn one", peer_id="web-visitor-bob"), + create_message("assistant", "ack", peer_id="web-visitor-bob"), + create_message("user", "peer turn two", peer_id="web-visitor-alice"), ] extract_ctx = create_mock_extract_context(messages) handler = MemoryIsolationHandler( ctx, extract_ctx, allow_self=True, - allowed_peer_ids={"web:visitor:alice", "web:visitor:bob"}, + allowed_peer_ids={"web-visitor-alice", "web-visitor-bob"}, ) from openviking.session.memory.dataclass import MemoryTypeSchema, ResolvedOperation @@ -523,7 +523,7 @@ def test_calculate_memory_uris_missing_peer_id_falls_back_to_first_peer_when_sel uris = handler.calculate_memory_uris(schema, operation, extract_ctx) assert uris == [ - "viking://user/support_bot/peers/web:visitor:bob/memories/preferences" + "viking://user/support_bot/peers/web-visitor-bob/memories/preferences" ] assert operation.memory_fields["user_id"] == "support_bot" - assert operation.memory_fields["peer_id"] == "web:visitor:bob" + assert operation.memory_fields["peer_id"] == "web-visitor-bob" diff --git a/tests/session/memory/test_memory_react_system_prompt.py b/tests/session/memory/test_memory_react_system_prompt.py index 874a98228c..61bf63eed8 100644 --- a/tests/session/memory/test_memory_react_system_prompt.py +++ b/tests/session/memory/test_memory_react_system_prompt.py @@ -111,14 +111,14 @@ def test_assemble_conversation_uses_peer_id_when_present(self): id="m1", role="user", parts=[TextPart("My invoice is still missing.")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) ] provider = SessionExtractContextProvider(messages=messages) conversation = provider._assemble_conversation(messages) - assert "[0][user][web:visitor:alice]" in conversation + assert "[0][user][web-visitor-alice]" in conversation assert "[0][user][default]" not in conversation def test_detect_language_only_uses_text_parts(self): diff --git a/tests/session/memory/test_memory_timestamp_parsing.py b/tests/session/memory/test_memory_timestamp_parsing.py index 2f05c26f6b..f081be83f3 100644 --- a/tests/session/memory/test_memory_timestamp_parsing.py +++ b/tests/session/memory/test_memory_timestamp_parsing.py @@ -82,13 +82,13 @@ def test_message_range_uses_peer_id_when_present(): id="msg-peer", role="user", parts=[TextPart(text="invoice follow-up")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) ] ] ) - assert "[web:visitor:alice]: invoice follow-up" in msg_range.pretty_print() + assert "[web-visitor-alice]: invoice follow-up" in msg_range.pretty_print() assert "[default]: invoice follow-up" not in msg_range.pretty_print() @@ -130,13 +130,13 @@ def test_message_range_merges_adjacent_chunks_for_same_speaker(): id="msg-long#chunk_0", role="user", parts=[TextPart(text="first chunk. ")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) second = Message( id="msg-long#chunk_1", role="user", parts=[TextPart(text="second chunk.")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) msg_range = MessageRange( [[first, second]], @@ -154,7 +154,7 @@ def test_message_range_merges_adjacent_chunks_for_same_speaker(): }, ) - assert msg_range.pretty_print() == "[web:visitor:alice]: first chunk. second chunk." + assert msg_range.pretty_print() == "[web-visitor-alice]: first chunk. second chunk." def test_message_range_keeps_regular_same_speaker_messages_separate(): @@ -165,20 +165,20 @@ def test_message_range_keeps_regular_same_speaker_messages_separate(): id="msg-a", role="user", parts=[TextPart(text="first message")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ), Message( id="msg-b", role="user", parts=[TextPart(text="second message")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ), ] ] ) assert msg_range.pretty_print() == ( - "[web:visitor:alice]: first message\n[web:visitor:alice]: second message" + "[web-visitor-alice]: first message\n[web-visitor-alice]: second message" ) @@ -187,13 +187,13 @@ def test_message_range_keeps_different_source_chunks_separate(): id="msg-a#chunk_0", role="user", parts=[TextPart(text="first source chunk")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) second = Message( id="msg-b#chunk_0", role="user", parts=[TextPart(text="second source chunk")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) msg_range = MessageRange( [[first, second]], @@ -212,7 +212,7 @@ def test_message_range_keeps_different_source_chunks_separate(): ) assert msg_range.pretty_print() == ( - "[web:visitor:alice]: first source chunk...\n[web:visitor:alice]: second source chunk..." + "[web-visitor-alice]: first source chunk...\n[web-visitor-alice]: second source chunk..." ) @@ -224,14 +224,14 @@ def test_message_range_does_not_treat_original_chunk_like_id_as_chunk(): id="msg-long#chunk_0", role="user", parts=[TextPart(text="original message id contains chunk marker")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ), ] ] ) assert msg_range.pretty_print() == ( - "[web:visitor:alice]: original message id contains chunk marker" + "[web-visitor-alice]: original message id contains chunk marker" ) @@ -240,7 +240,7 @@ def test_message_range_marks_middle_chunk_as_abbreviated(): id="msg-long#chunk_1", role="user", parts=[TextPart(text="middle chunk")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) msg_range = MessageRange( [[message]], @@ -253,7 +253,7 @@ def test_message_range_marks_middle_chunk_as_abbreviated(): }, ) - assert msg_range.pretty_print() == "[web:visitor:alice]: ...middle chunk..." + assert msg_range.pretty_print() == "[web-visitor-alice]: ...middle chunk..." def test_peer_id_routes_peer_memory_for_all_role_selected_types(stub_provider_config): @@ -262,7 +262,7 @@ def test_peer_id_routes_peer_memory_for_all_role_selected_types(stub_provider_co id="msg-peer", role="user", parts=[TextPart(text="I am Alice. Please contact me by email for invoices.")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) ] extract_context = ExtractContext(messages) @@ -273,7 +273,7 @@ def test_peer_id_routes_peer_memory_for_all_role_selected_types(stub_provider_co handler = MemoryIsolationHandler( ctx, extract_context, - allowed_peer_ids={"web:visitor:alice"}, + allowed_peer_ids={"web-visitor-alice"}, ) role_scope = handler.get_read_scope() fields = {"ranges": "0"} @@ -290,7 +290,7 @@ def test_peer_id_routes_peer_memory_for_all_role_selected_types(stub_provider_co ResolvedOperation(memory_fields=fields, memory_type="profile", uris=[]), extract_context, ) - assert profile_uris == ["viking://user/support_bot/peers/web:visitor:alice/memories/profile.md"] + assert profile_uris == ["viking://user/support_bot/peers/web-visitor-alice/memories/profile.md"] tool_schema = MemoryTypeSchema( memory_type="tools", @@ -305,7 +305,7 @@ def test_peer_id_routes_peer_memory_for_all_role_selected_types(stub_provider_co extract_context, ) assert tool_uris == [ - "viking://user/support_bot/peers/web:visitor:alice/memories/tools/email.md" + "viking://user/support_bot/peers/web-visitor-alice/memories/tools/email.md" ] @@ -315,7 +315,7 @@ def test_peer_id_range_does_not_route_without_allowed_peer_ids(stub_provider_con id="msg-peer", role="user", parts=[TextPart(text="I am Alice. Please contact me by email for invoices.")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) ] extract_context = ExtractContext(messages) diff --git a/tests/session/test_session_commit.py b/tests/session/test_session_commit.py index 2a9748c8e3..98e0f40dc7 100644 --- a/tests/session/test_session_commit.py +++ b/tests/session/test_session_commit.py @@ -222,12 +222,12 @@ async def fake_execution_extract( session.add_message( "user", [TextPart("我是 Alice，后续发票问题请优先邮件联系我，邮箱是 alice@example.com。")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) session.add_message( "assistant", [TextPart("收到，我会优先通过邮件联系你，并继续跟进发票问题。")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) session._meta.memory_policy = { @@ -247,9 +247,9 @@ async def fake_execution_extract( "profile", }, "allow_self_memory": False, - "allowed_peer_ids": {"web:visitor:alice"}, + "allowed_peer_ids": {"web-visitor-alice"}, "roles": ["user", "assistant"], - "peer_ids": ["web:visitor:alice", "web:visitor:alice"], + "peer_ids": ["web-visitor-alice", "web-visitor-alice"], }, ] assert execution_calls == [] diff --git a/tests/session/test_session_messages.py b/tests/session/test_session_messages.py index e06552fc54..ce8aa86ae5 100644 --- a/tests/session/test_session_messages.py +++ b/tests/session/test_session_messages.py @@ -71,11 +71,11 @@ async def test_add_message_with_peer_id(self, session: Session): msg = session.add_message( "user", [TextPart("Message from Alice")], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) - assert msg.peer_id == "web:visitor:alice" - assert msg.to_dict()["peer_id"] == "web:visitor:alice" + assert msg.peer_id == "web-visitor-alice" + assert msg.to_dict()["peer_id"] == "web-visitor-alice" async def test_add_message_rejects_peer_id_with_path_separator(self, session: Session): """Test direct session usage validates peer_id path safety.""" diff --git a/tests/session/test_tool_result_externalization.py b/tests/session/test_tool_result_externalization.py index c7f3a04082..bf8f73014d 100644 --- a/tests/session/test_tool_result_externalization.py +++ b/tests/session/test_tool_result_externalization.py @@ -97,7 +97,7 @@ async def test_add_message_externalizes_large_tool_output(session: Session): tool_status="completed", ) ], - peer_id="web:visitor:alice", + peer_id="web-visitor-alice", ) part = msg.get_tool_parts()[0] @@ -111,7 +111,7 @@ async def test_add_message_externalizes_large_tool_output(session: Session): assert stored["content"] == raw assert stored["offset_unit"] == "unicode_code_point" assert stored["metadata"]["user_id"] == session.ctx.user.user_id - assert stored["metadata"]["peer_id"] == "web:visitor:alice" + assert stored["metadata"]["peer_id"] == "web-visitor-alice" async def test_hydrate_tool_outputs_for_extraction_uses_memory_copy(session: Session): diff --git a/tests/unit/test_namespace_uri_classification.py b/tests/unit/test_namespace_uri_classification.py index 079ae96269..27eda8a12d 100644 --- a/tests/unit/test_namespace_uri_classification.py +++ b/tests/unit/test_namespace_uri_classification.py @@ -2,17 +2,23 @@ # SPDX-License-Identifier: Apache-2.0 """Tests for shared Viking URI namespace/content classification.""" +import pytest + from openviking.core.namespace import ( canonical_session_uri, canonicalize_uri, classify_uri, context_type_for_uri, + is_content_namespace_root_uri, + is_content_root_uri, is_session_uri, legacy_session_uri, owner_space_for_uri, visible_roots, ) +from openviking.core.uri_validation import validate_content_target_uri from openviking.server.identity import RequestContext, Role +from openviking_cli.exceptions import InvalidURIError from openviking_cli.session.user_id import UserIdentifier @@ -23,14 +29,52 @@ def test_context_type_for_uri_uses_path_segments(): assert context_type_for_uri("viking://user/skills/demo") == "skill" assert ( context_type_for_uri( - "viking://user/support_bot/peers/web:visitor:alice/memories/profile.md" + "viking://user/support_bot/peers/web-visitor-alice/memories/profile.md" ) == "memory" ) + assert ( + context_type_for_uri("viking://user/support_bot/peers/web-visitor-alice/resources/faq.md") + == "resource" + ) + assert ( + context_type_for_uri( + "viking://user/support_bot/peers/web-visitor-alice/skills/search-helper" + ) + == "skill" + ) assert context_type_for_uri("viking://resources/memories-report.md") == "resource" assert context_type_for_uri("viking://user/alice/resources/skills-report.md") == "resource" +def test_peer_content_target_uri_rejects_invalid_peer_id(): + ctx = RequestContext( + user=UserIdentifier(account_id="acct", user_id="support_bot"), + role=Role.USER, + ) + + with pytest.raises(InvalidURIError, match="Invalid peer_id"): + validate_content_target_uri( + "viking://user/support_bot/peers/web:visitor:alice/skills/demo", + ctx, + kind="skill", + ) + + +def test_user_content_target_uri_rejects_invalid_user_id(): + ctx = RequestContext( + user=UserIdentifier(account_id="acct", user_id="support_bot"), + role=Role.ROOT, + ) + + with pytest.raises(InvalidURIError, match="Invalid user_id"): + validate_content_target_uri( + "viking://user/team:alice/resources/demo", + ctx, + kind="resource", + ) + + def test_exact_memory_and_skill_root_detection(): assert classify_uri("viking://user/alice/memories/preferences/prefs.md").is_memory assert classify_uri("viking://user/alice/memories").is_memory_root @@ -75,3 +119,42 @@ def test_session_uri_helpers_use_user_namespace(): assert is_session_uri("viking://user/sessions/s1") assert is_session_uri("viking://session/s1") assert "viking://session" not in visible_roots(ctx) + + +def test_current_user_short_content_roots_are_canonicalized_from_content_segments(): + ctx = RequestContext( + user=UserIdentifier(account_id="acct", user_id="alice"), + role=Role.USER, + ) + + assert canonicalize_uri("viking://user/resources", ctx) == "viking://user/alice/resources" + assert ( + canonicalize_uri("viking://user/resources/docs", ctx) + == "viking://user/alice/resources/docs" + ) + assert ( + validate_content_target_uri( + "viking://user/resources", + ctx, + kind="resource", + field_name="parent", + ) + == "viking://user/alice/resources" + ) + assert canonicalize_uri("viking://user/alice/resources", ctx) == ( + "viking://user/alice/resources" + ) + assert is_content_namespace_root_uri("viking://user/resources", ctx) + assert is_content_namespace_root_uri("viking://user/resources/", ctx) + assert is_content_namespace_root_uri("viking://resources", ctx) + assert is_content_root_uri("viking://resources", ctx, kind="resource") + assert not is_content_namespace_root_uri("viking://user/resources/docs", ctx) + + +def test_user_root_short_form_uses_current_identity(): + ctx = RequestContext( + user=UserIdentifier(account_id="acct", user_id="alice"), + role=Role.ROOT, + ) + + assert canonicalize_uri("viking://user", ctx) == "viking://user/alice" From 585639c8f0105d326451e5567026c1887e801a6b Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Thu, 11 Jun 2026 22:14:53 +0800 Subject: [PATCH 2/7] fix(core): handle scoped skill updates --- bot/vikingbot/openviking_mount/ov_server.py | 18 ++++++++++++------ .../unit/test_openviking_peer_identity.py | 14 ++++++++------ openviking/server/routers/skills.py | 3 +-- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/bot/vikingbot/openviking_mount/ov_server.py b/bot/vikingbot/openviking_mount/ov_server.py index 5f9eb3e807..4dc39b53fe 100644 --- a/bot/vikingbot/openviking_mount/ov_server.py +++ b/bot/vikingbot/openviking_mount/ov_server.py @@ -1,4 +1,5 @@ import asyncio +import base64 import json import re import uuid @@ -7,6 +8,7 @@ from loguru import logger import openviking as ov +from openviking.core.peer_id import safe_peer_id as openviking_safe_peer_id from vikingbot.config.loader import load_config from vikingbot.openviking_mount.user_apikey_manager import UserApiKeyManager @@ -20,9 +22,16 @@ def _is_session_key(agent_id: Optional[str]) -> bool: def _safe_peer_id(peer_id: Optional[str]) -> Optional[str]: if not peer_id: return None - if "/" in peer_id or "\\" in peer_id: + raw_peer_id = str(peer_id).strip() + if not raw_peer_id: return None - return peer_id + if "/" in raw_peer_id or "\\" in raw_peer_id: + return None + if safe_peer_id := openviking_safe_peer_id(raw_peer_id): + return safe_peer_id + + encoded = base64.urlsafe_b64encode(raw_peer_id.encode("utf-8")).decode("ascii").rstrip("=") + return openviking_safe_peer_id(f"ext-{encoded}") class VikingClient: @@ -420,9 +429,6 @@ def build_memory_search_requests( def _skill_memory_uri(self, skill_name: str, user_id: Optional[str] = None) -> str: return f"{self._memory_target_uri(user_id)}skills/{skill_name}.md" - def should_sender_fanout(self) -> bool: - return self._is_root_key_mode() - async def find( self, query: str, @@ -1019,7 +1025,7 @@ async def commit( appended = await self.append_messages( session_id, messages, - default_user_peer_id=peer_id, + default_user_peer_id=self._peer_id(peer_id), session_user_id=session_user_id, ) commit_result = await self.commit_session( diff --git a/bot/vikingbot/tests/unit/test_openviking_peer_identity.py b/bot/vikingbot/tests/unit/test_openviking_peer_identity.py index 49f340aac0..98f2efc649 100644 --- a/bot/vikingbot/tests/unit/test_openviking_peer_identity.py +++ b/bot/vikingbot/tests/unit/test_openviking_peer_identity.py @@ -13,7 +13,9 @@ sys.modules.setdefault("vikingbot.config", config_module) sys.modules.setdefault("vikingbot.config.loader", loader_module) -from vikingbot.openviking_mount.ov_server import VikingClient +from vikingbot.openviking_mount.ov_server import VikingClient # noqa: E402 + +TELEGRAM_ALICE_PEER_ID = "ext-dGVsZWdyYW06YWxpY2U" def _client(api_key_type: str = "user") -> VikingClient: @@ -40,7 +42,7 @@ def test_normalize_session_messages_maps_sender_to_peer_only_for_user_messages() normalized = client._normalize_session_messages(messages) - assert normalized[0]["peer_id"] == "telegram:alice" + assert normalized[0]["peer_id"] == TELEGRAM_ALICE_PEER_ID assert "peer_id" not in normalized[1] @@ -68,7 +70,7 @@ async def fake_read_content(uri, level="read"): profile = await client.read_peer_profile("telegram:alice") assert profile == "Alice profile" - assert calls == [("viking://user/peers/telegram:alice/memories/profile.md", "read")] + assert calls == [(f"viking://user/peers/{TELEGRAM_ALICE_PEER_ID}/memories/profile.md", "read")] @pytest.mark.asyncio @@ -86,7 +88,7 @@ async def fake_read_content(uri, level="read"): assert profile == "Alice profile" assert calls == [ - ("viking://user/bot-user/peers/telegram:alice/memories/profile.md", "read") + (f"viking://user/bot-user/peers/{TELEGRAM_ALICE_PEER_ID}/memories/profile.md", "read") ] @@ -147,7 +149,7 @@ async def fake_commit_session( ) assert calls["append"]["session_user_id"] is None - assert calls["append"]["default_user_peer_id"] == "telegram:alice" + assert calls["append"]["default_user_peer_id"] == TELEGRAM_ALICE_PEER_ID assert calls["commit"]["user_id"] is None assert calls["commit"]["memory_policy"] is None @@ -185,7 +187,7 @@ async def fake_commit_session( ) assert calls["append"]["session_user_id"] == "bot-user" - assert calls["append"]["default_user_peer_id"] == "telegram:alice" + assert calls["append"]["default_user_peer_id"] == TELEGRAM_ALICE_PEER_ID assert calls["commit"]["user_id"] == "bot-user" assert calls["commit"]["memory_policy"] is None diff --git a/openviking/server/routers/skills.py b/openviking/server/routers/skills.py index be65941a4d..fb6db64898 100644 --- a/openviking/server/routers/skills.py +++ b/openviking/server/routers/skills.py @@ -560,12 +560,11 @@ async def update_skill( http_request: Request, request: UpdateSkillRequest, skill_name: str = ApiPath(..., description="Skill name"), - root_uri: Optional[str] = None, _ctx: RequestContext = Depends(get_request_context), ): """Replace an existing agent skill with new content.""" service = get_service() - skills_root_uri = _resolve_skills_root(_ctx, root_uri) + skills_root_uri = _resolve_skills_root(_ctx, request.root_uri) skill_root_uri = await _require_skill(service, _ctx, skill_name, skills_root_uri) data = request.data From 203ab85c8a3a3a52fe0115446f37e9407184bce4 Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Fri, 12 Jun 2026 11:48:56 +0800 Subject: [PATCH 3/7] fix(core): keep skills user scoped --- bot/vikingbot/openviking_mount/ov_server.py | 32 ++--- .../unit/test_openviking_peer_identity.py | 2 +- crates/ov_cli/src/client.rs | 86 ++----------- crates/ov_cli/src/commands/resources.rs | 15 +-- crates/ov_cli/src/commands/skills.rs | 58 +++------ crates/ov_cli/src/handlers.rs | 15 --- crates/ov_cli/src/main.rs | 118 ++---------------- openviking/async_client.py | 6 - openviking/client/local.py | 6 - openviking/core/namespace.py | 5 +- openviking/core/peer_id.py | 8 -- openviking/core/retrieval_targets.py | 17 ++- openviking/server/routers/resources.py | 9 -- openviking/server/routers/skills.py | 82 ++++-------- openviking/service/resource_service.py | 12 -- .../memory/memory_isolation_handler.py | 21 ++-- openviking/session/session.py | 4 +- openviking/sync_client.py | 6 - openviking/utils/skill_processor.py | 53 +------- openviking_cli/client/base.py | 3 - openviking_cli/client/http.py | 6 - openviking_cli/client/sync_http.py | 6 - tests/server/test_api_skills.py | 65 ---------- tests/server/test_peer_id_compat.py | 9 +- .../unit/test_namespace_uri_classification.py | 10 +- 25 files changed, 117 insertions(+), 537 deletions(-) diff --git a/bot/vikingbot/openviking_mount/ov_server.py b/bot/vikingbot/openviking_mount/ov_server.py index 4dc39b53fe..14238fe0af 100644 --- a/bot/vikingbot/openviking_mount/ov_server.py +++ b/bot/vikingbot/openviking_mount/ov_server.py @@ -8,7 +8,7 @@ from loguru import logger import openviking as ov -from openviking.core.peer_id import safe_peer_id as openviking_safe_peer_id +from openviking.core.peer_id import normalize_peer_id from vikingbot.config.loader import load_config from vikingbot.openviking_mount.user_apikey_manager import UserApiKeyManager @@ -19,7 +19,7 @@ def _is_session_key(agent_id: Optional[str]) -> bool: return agent_id is not None and "__" in agent_id -def _safe_peer_id(peer_id: Optional[str]) -> Optional[str]: +def _peer_id_from_external_id(peer_id: Optional[str]) -> Optional[str]: if not peer_id: return None raw_peer_id = str(peer_id).strip() @@ -27,11 +27,13 @@ def _safe_peer_id(peer_id: Optional[str]) -> Optional[str]: return None if "/" in raw_peer_id or "\\" in raw_peer_id: return None - if safe_peer_id := openviking_safe_peer_id(raw_peer_id): - return safe_peer_id + try: + return normalize_peer_id(raw_peer_id) + except ValueError: + pass encoded = base64.urlsafe_b64encode(raw_peer_id.encode("utf-8")).decode("ascii").rstrip("=") - return openviking_safe_peer_id(f"ext-{encoded}") + return normalize_peer_id(f"ext-{encoded}") class VikingClient: @@ -248,7 +250,7 @@ def default_memory_policy() -> Dict[str, Dict[str, bool]]: @staticmethod def _peer_id(value: Optional[str]) -> Optional[str]: - return _safe_peer_id(str(value)) if value is not None else None + return _peer_id_from_external_id(str(value)) if value is not None else None async def _load_namespace_policy(self) -> None: if self._namespace_policy_loaded: @@ -340,9 +342,9 @@ def build_current_memory_target_uris( normalized_peer_ids = self._dedupe_strings( [ - safe_peer_id - for safe_peer_id in (self._peer_id(peer_id) for peer_id in (peer_ids or [])) - if safe_peer_id + pid + for pid in (self._peer_id(peer_id) for peer_id in (peer_ids or [])) + if pid ] ) for peer_id in normalized_peer_ids: @@ -377,9 +379,9 @@ def build_memory_search_requests( ) normalized_peer_ids = self._dedupe_strings( [ - safe_peer_id - for safe_peer_id in (self._peer_id(peer_id) for peer_id in (peer_ids or [])) - if safe_peer_id + pid + for pid in (self._peer_id(peer_id) for peer_id in (peer_ids or [])) + if pid ] ) effective_owner_user_id = self._effective_user_id(owner_user_id) if owner_user_id else None @@ -701,9 +703,9 @@ def _extract_memories(result: Any) -> list[Any]: peer_values.append(peer_id) normalized_peer_ids = self._dedupe_strings( [ - safe_peer_id - for safe_peer_id in (self._peer_id(peer_value) for peer_value in peer_values) - if safe_peer_id + pid + for pid in (self._peer_id(peer_value) for peer_value in peer_values) + if pid ] ) effective_owner_user_id = self._effective_user_id(owner_user_id) if owner_user_id else None diff --git a/bot/vikingbot/tests/unit/test_openviking_peer_identity.py b/bot/vikingbot/tests/unit/test_openviking_peer_identity.py index 98f2efc649..fa4ab7a831 100644 --- a/bot/vikingbot/tests/unit/test_openviking_peer_identity.py +++ b/bot/vikingbot/tests/unit/test_openviking_peer_identity.py @@ -37,7 +37,7 @@ def test_normalize_session_messages_maps_sender_to_peer_only_for_user_messages() messages = [ {"role": "user", "content": "hello", "sender_id": "telegram:alice"}, - {"role": "assistant", "content": "hi", "sender_id": "telegram:alice"}, + {"role": "assistant", "content": "hi", "sender_id": "agent-1"}, ] normalized = client._normalize_session_messages(messages) diff --git a/crates/ov_cli/src/client.rs b/crates/ov_cli/src/client.rs index c1ee22fe5e..6f228e68fa 100644 --- a/crates/ov_cli/src/client.rs +++ b/crates/ov_cli/src/client.rs @@ -625,9 +625,6 @@ impl HttpClient { pub async fn add_skill( &self, data: &str, - to: Option, - parent: Option, - parent_auto_create: Option, wait: bool, timeout: Option, show_progress: bool, @@ -635,31 +632,6 @@ impl HttpClient { source_metadata: Option, ) -> Result { let path_obj = Path::new(data); - let (effective_parent, create_parent) = match (parent, parent_auto_create) { - (Some(p), None) => (Some(p), false), - (None, Some(p)) => (Some(p), true), - (None, None) => (None, false), - (Some(_), Some(_)) => { - return Err(Error::Client( - "Specify only one of --parent or --parent-auto-create.".to_string(), - )); - } - }; - if to.is_some() && effective_parent.is_some() { - return Err(Error::Client( - "Specify only one of --to, --parent, or --parent-auto-create.".to_string(), - )); - } - - let build_body = |base: serde_json::Value| { - let mut body = base; - if create_parent { - body.as_object_mut() - .expect("add_skill request body must be an object") - .insert("create_parent".to_string(), serde_json::Value::Bool(true)); - } - body - }; if path_obj.exists() { if path_obj.is_dir() { @@ -675,13 +647,11 @@ impl HttpClient { self.upload_temp_file(zip_file.path()).await? }; - let mut body = build_body(serde_json::json!({ + let mut body = serde_json::json!({ "temp_file_id": temp_file_id, - "to": to, - "parent": effective_parent, "wait": wait, "timeout": timeout, - })); + }); if let Some(source_metadata) = source_metadata.clone() { body["source_metadata"] = source_metadata; } @@ -698,13 +668,11 @@ impl HttpClient { self.upload_temp_file(path_obj).await? }; - let mut body = build_body(serde_json::json!({ + let mut body = serde_json::json!({ "temp_file_id": temp_file_id, - "to": to, - "parent": effective_parent, "wait": wait, "timeout": timeout, - })); + }); if let Some(source_metadata) = source_metadata.clone() { body["source_metadata"] = source_metadata; } @@ -714,26 +682,22 @@ impl HttpClient { .post_with_timeout("/api/v1/skills", &body, dynamic_timeout) .await } else { - let mut body = build_body(serde_json::json!({ + let mut body = serde_json::json!({ "data": data, - "to": to, - "parent": effective_parent, "wait": wait, "timeout": timeout, - })); + }); if let Some(source_metadata) = source_metadata.clone() { body["source_metadata"] = source_metadata; } self.post("/api/v1/skills", &body).await } } else { - let mut body = build_body(serde_json::json!({ + let mut body = serde_json::json!({ "data": data, - "to": to, - "parent": effective_parent, "wait": wait, "timeout": timeout, - })); + }); if let Some(source_metadata) = source_metadata { body["source_metadata"] = source_metadata; } @@ -741,15 +705,8 @@ impl HttpClient { } } - pub async fn skills_list( - &self, - node_limit: i32, - root_uri: Option<&str>, - ) -> Result { - let mut params = vec![("node_limit".to_string(), node_limit.to_string())]; - if let Some(root_uri) = root_uri { - params.push(("root_uri".to_string(), root_uri.to_string())); - } + pub async fn skills_list(&self, node_limit: i32) -> Result { + let params = vec![("node_limit".to_string(), node_limit.to_string())]; self.get("/api/v1/skills", ¶ms).await } @@ -760,7 +717,6 @@ impl HttpClient { include_files: bool, include_source: bool, level: Option, - root_uri: Option<&str>, ) -> Result { let path = format!("/api/v1/skills/{}", name); let mut params = vec![ @@ -771,9 +727,6 @@ impl HttpClient { if let Some(level) = level { params.push(("level".to_string(), level.to_string())); } - if let Some(root_uri) = root_uri { - params.push(("root_uri".to_string(), root_uri.to_string())); - } self.get(&path, ¶ms).await } @@ -783,11 +736,9 @@ impl HttpClient { node_limit: i32, threshold: Option, level: Option>, - root_uri: Option<&str>, ) -> Result { let body = serde_json::json!({ "query": query, - "root_uri": root_uri, "limit": node_limit, "score_threshold": threshold, "level": level, @@ -858,7 +809,6 @@ impl HttpClient { show_progress: bool, verbose: bool, source_metadata: Option, - root_uri: Option<&str>, ) -> Result { let endpoint = format!("/api/v1/skills/{}", name); let path_obj = Path::new(data); @@ -878,7 +828,6 @@ impl HttpClient { }; let mut body = serde_json::json!({ "temp_file_id": temp_file_id, - "root_uri": root_uri, "wait": wait, "timeout": timeout, }); @@ -895,7 +844,6 @@ impl HttpClient { }; let mut body = serde_json::json!({ "temp_file_id": temp_file_id, - "root_uri": root_uri, "wait": wait, "timeout": timeout, }); @@ -906,7 +854,6 @@ impl HttpClient { } else { let mut body = serde_json::json!({ "data": data, - "root_uri": root_uri, "wait": wait, "timeout": timeout, }); @@ -918,7 +865,6 @@ impl HttpClient { } else { let mut body = serde_json::json!({ "data": data, - "root_uri": root_uri, "wait": wait, "timeout": timeout, }); @@ -929,17 +875,9 @@ impl HttpClient { } } - pub async fn skill_remove( - &self, - name: &str, - root_uri: Option<&str>, - ) -> Result { + pub async fn skill_remove(&self, name: &str) -> Result { let path = format!("/api/v1/skills/{}", name); - let mut params = Vec::new(); - if let Some(root_uri) = root_uri { - params.push(("root_uri".to_string(), root_uri.to_string())); - } - self.delete(&path, ¶ms).await + self.delete(&path, &[]).await } // ============ Task Methods ============ diff --git a/crates/ov_cli/src/commands/resources.rs b/crates/ov_cli/src/commands/resources.rs index 69698dc3fc..ca9c404cdc 100644 --- a/crates/ov_cli/src/commands/resources.rs +++ b/crates/ov_cli/src/commands/resources.rs @@ -58,9 +58,6 @@ pub async fn add_resource( pub async fn add_skill( client: &HttpClient, data: &str, - to: Option, - parent: Option, - parent_auto_create: Option, wait: bool, timeout: Option, show_progress: bool, @@ -69,17 +66,7 @@ pub async fn add_skill( compact: bool, ) -> Result<()> { let result = client - .add_skill( - data, - to, - parent, - parent_auto_create, - wait, - timeout, - show_progress, - verbose, - None, - ) + .add_skill(data, wait, timeout, show_progress, verbose, None) .await?; if !wait && matches!(format, OutputFormat::Table) { diff --git a/crates/ov_cli/src/commands/skills.rs b/crates/ov_cli/src/commands/skills.rs index 06e2601312..c1685b6673 100644 --- a/crates/ov_cli/src/commands/skills.rs +++ b/crates/ov_cli/src/commands/skills.rs @@ -117,9 +117,6 @@ pub async fn add( let result = client .add_skill( &target.data, - None, - None, - None, wait, None, show_progress, @@ -155,12 +152,11 @@ pub async fn add( pub async fn list( client: &HttpClient, - root_uri: Option<&str>, node_limit: i32, output_format: OutputFormat, compact: bool, ) -> Result<()> { - let result = client.skills_list(node_limit, root_uri).await?; + let result = client.skills_list(node_limit).await?; output_success(result, output_format, compact); Ok(()) } @@ -168,7 +164,6 @@ pub async fn list( pub async fn show( client: &HttpClient, name: &str, - root_uri: Option<&str>, level: Option, include_files: bool, include_source: bool, @@ -177,14 +172,7 @@ pub async fn show( ) -> Result<()> { let include_content = level.is_none() || level == Some(2); let mut result = client - .skill_show( - name, - include_content, - include_files, - include_source, - level, - root_uri, - ) + .skill_show(name, include_content, include_files, include_source, level) .await?; if let Some(level) = level { filter_skill_show_level(&mut result, level); @@ -196,7 +184,6 @@ pub async fn show( pub async fn find( client: &HttpClient, query: &str, - root_uri: Option<&str>, node_limit: i32, threshold: Option, level: Option>, @@ -204,7 +191,7 @@ pub async fn find( compact: bool, ) -> Result<()> { let result = client - .skill_find(query, node_limit, threshold, level, root_uri) + .skill_find(query, node_limit, threshold, level) .await?; output_success(result, output_format, compact); Ok(()) @@ -212,7 +199,6 @@ pub async fn find( pub async fn update( client: &HttpClient, - root_uri: Option<&str>, skill_names: Vec, wait: bool, yes: bool, @@ -220,7 +206,7 @@ pub async fn update( compact: bool, ) -> Result<()> { let update_all = skill_names.is_empty(); - let names = resolve_installed_skill_names(client, root_uri, skill_names).await?; + let names = resolve_installed_skill_names(client, skill_names).await?; if names.is_empty() { output_message_result( serde_json::json!({ "updated": [], "total": 0 }), @@ -243,8 +229,7 @@ pub async fn update( let mut updated = Vec::new(); let mut skipped = Vec::new(); for name in names { - let update_target = match resolve_update_target(client, root_uri, &name, !update_all).await - { + let update_target = match resolve_update_target(client, &name, !update_all).await { Ok(target) => target, Err(error) if update_all => { skipped.push(json!({ @@ -265,7 +250,6 @@ pub async fn update( false, false, source_metadata, - root_uri, ) .await?; updated.push(result); @@ -287,7 +271,6 @@ pub async fn update( pub async fn remove( client: &HttpClient, - root_uri: Option<&str>, skill_names: Vec, all: bool, yes: bool, @@ -301,7 +284,7 @@ pub async fn remove( } let requested_names = normalize_skill_names(skill_names)?; let names = if all { - resolve_installed_skill_names(client, root_uri, Vec::new()).await? + resolve_installed_skill_names(client, Vec::new()).await? } else if !requested_names.is_empty() { requested_names } else { @@ -310,7 +293,7 @@ pub async fn remove( "Specify at least one skill name, or pass --all.".to_string(), )); } - match prompt_remove_skill_selection(client, root_uri).await? { + match prompt_remove_skill_selection(client).await? { Some(selected) => selected, None => { output_message_result( @@ -345,7 +328,7 @@ pub async fn remove( let removed_names = names.clone(); let mut removed = Vec::new(); for name in names { - removed.push(client.skill_remove(&name, root_uri).await?); + removed.push(client.skill_remove(&name).await?); } let total = removed.len(); output_message_result( @@ -1151,11 +1134,10 @@ fn skill_target_label(target: &AddTarget) -> String { async fn resolve_update_target( client: &HttpClient, - root_uri: Option<&str>, name: &str, allow_prompt: bool, ) -> Result { - if let Some(record) = read_skill_source_record(client, root_uri, name).await? { + if let Some(record) = read_skill_source_record(client, name).await? { return update_target_from_record(&record, name, allow_prompt); } @@ -1173,12 +1155,9 @@ async fn resolve_update_target( async fn read_skill_source_record( client: &HttpClient, - root_uri: Option<&str>, name: &str, ) -> Result> { - let result = client - .skill_show(name, false, false, true, Some(0), root_uri) - .await?; + let result = client.skill_show(name, false, false, true, Some(0)).await?; let Some(source) = result.get("source") else { return Ok(None); }; @@ -1353,7 +1332,6 @@ fn resolve_named_skill_dir(root: &Path, name: &str) -> Result { async fn resolve_installed_skill_names( client: &HttpClient, - root_uri: Option<&str>, requested: Vec, ) -> Result> { let requested = normalize_skill_names(requested)?; @@ -1361,18 +1339,15 @@ async fn resolve_installed_skill_names( return Ok(requested); } - Ok(list_installed_skills(client, root_uri) + Ok(list_installed_skills(client) .await? .into_iter() .map(|skill| skill.name) .collect()) } -async fn list_installed_skills( - client: &HttpClient, - root_uri: Option<&str>, -) -> Result> { - let result = client.skills_list(10000, root_uri).await?; +async fn list_installed_skills(client: &HttpClient) -> Result> { + let result = client.skills_list(10000).await?; let skills = result .get("skills") .and_then(Value::as_array) @@ -1392,11 +1367,8 @@ async fn list_installed_skills( Ok(skills) } -async fn prompt_remove_skill_selection( - client: &HttpClient, - root_uri: Option<&str>, -) -> Result>> { - let skills = list_installed_skills(client, root_uri).await?; +async fn prompt_remove_skill_selection(client: &HttpClient) -> Result>> { + let skills = list_installed_skills(client).await?; if skills.is_empty() { return Ok(Some(Vec::new())); } diff --git a/crates/ov_cli/src/handlers.rs b/crates/ov_cli/src/handlers.rs index 6242ebff05..f1e9d10390 100644 --- a/crates/ov_cli/src/handlers.rs +++ b/crates/ov_cli/src/handlers.rs @@ -110,29 +110,14 @@ pub async fn handle_add_resource( pub async fn handle_add_skill( data: String, - to: Option, - parent: Option, - parent_auto_create: Option, wait: bool, timeout: Option, ctx: CliContext, ) -> Result<()> { - let exclusive_count = usize::from(to.is_some()) - + usize::from(parent.is_some()) - + usize::from(parent_auto_create.is_some()); - if exclusive_count > 1 { - return Err(Error::Client( - "Specify only one of --to, --parent, or --parent-auto-create.".to_string(), - )); - } - let client = ctx.get_client(); commands::resources::add_skill( &client, &data, - to, - parent, - parent_auto_create, wait, timeout, ctx.should_show_progress(), diff --git a/crates/ov_cli/src/main.rs b/crates/ov_cli/src/main.rs index 5b472c0bfc..4f77b86ced 100644 --- a/crates/ov_cli/src/main.rs +++ b/crates/ov_cli/src/main.rs @@ -268,15 +268,6 @@ enum Commands { AddSkill { /// Skill directory, SKILL.md, or raw content data: String, - /// Exact target URI (must end with the skill name) (cannot be used with --parent) - #[arg(long)] - to: Option, - /// Target parent skills URI (must already exist and be a directory) (cannot be used with --to) - #[arg(long)] - parent: Option, - /// Target parent skills URI (create parent directory if it does not exist) (cannot be used with --to or --parent) - #[arg(short = 'p', long = "parent-auto-create")] - parent_auto_create: Option, /// Wait until processing is complete #[arg(long)] wait: bool, @@ -888,9 +879,6 @@ enum SkillCommands { /// List installed agent skills #[command(alias = "ls")] List { - /// Skills root URI to manage - #[arg(long = "root-uri")] - root_uri: Option, /// Maximum number of skills to list #[arg( short = 'n', @@ -904,9 +892,6 @@ enum SkillCommands { Find { /// Search query query: String, - /// Skills root URI to search - #[arg(long = "root-uri")] - root_uri: Option, /// Maximum number of results #[arg( short = 'n', @@ -926,9 +911,6 @@ enum SkillCommands { Show { /// Skill name name: String, - /// Skills root URI containing the skill - #[arg(long = "root-uri")] - root_uri: Option, /// Detail level to show (0=abstract, 1=overview, 2=SKILL.md content) #[arg(short = 'L', long = "level", value_parser = clap::value_parser!(i32).range(0..=2))] level: Option, @@ -947,9 +929,6 @@ enum SkillCommands { }, /// Update installed skills from their recorded source Update { - /// Skills root URI to update from - #[arg(long = "root-uri")] - root_uri: Option, /// Skill name(s) to update; omit to update all installed skills skills: Vec, /// Wait until processing is complete @@ -962,9 +941,6 @@ enum SkillCommands { /// Remove installed skills #[command(alias = "rm", alias = "delete")] Remove { - /// Skills root URI to remove from - #[arg(long = "root-uri")] - root_uri: Option, /// Skill name(s) to remove skills: Vec, /// Remove all installed skills @@ -2213,17 +2189,13 @@ async fn main() { } Commands::AddSkill { data, - to, - parent, - parent_auto_create, wait, timeout, upload_options, } => { let ctx = ctx.with_upload_options(upload_options.merged_with_legacy(legacy_upload_options)); - handlers::handle_add_skill(data, to, parent, parent_auto_create, wait, timeout, ctx) - .await + handlers::handle_add_skill(data, wait, timeout, ctx).await } Commands::Skills { action } => match action { SkillCommands::Add { @@ -2248,23 +2220,12 @@ async fn main() { ) .await } - SkillCommands::List { - root_uri, - node_limit, - } => { + SkillCommands::List { node_limit } => { let client = ctx.get_client(); - commands::skills::list( - &client, - root_uri.as_deref(), - node_limit, - ctx.output_format, - ctx.compact, - ) - .await + commands::skills::list(&client, node_limit, ctx.output_format, ctx.compact).await } SkillCommands::Find { query, - root_uri, node_limit, threshold, level, @@ -2273,7 +2234,6 @@ async fn main() { commands::skills::find( &client, &query, - root_uri.as_deref(), node_limit, threshold, level, @@ -2284,7 +2244,6 @@ async fn main() { } SkillCommands::Show { name, - root_uri, level, files, source, @@ -2300,7 +2259,6 @@ async fn main() { commands::skills::show( &client, &name, - root_uri.as_deref(), level, files, source, @@ -2309,41 +2267,15 @@ async fn main() { ) .await } - SkillCommands::Update { - root_uri, - skills, - wait, - yes, - } => { + SkillCommands::Update { skills, wait, yes } => { let client = ctx.get_client(); - commands::skills::update( - &client, - root_uri.as_deref(), - skills, - wait, - yes, - ctx.output_format, - ctx.compact, - ) - .await + commands::skills::update(&client, skills, wait, yes, ctx.output_format, ctx.compact) + .await } - SkillCommands::Remove { - root_uri, - skills, - all, - yes, - } => { + SkillCommands::Remove { skills, all, yes } => { let client = ctx.get_client(); - commands::skills::remove( - &client, - root_uri.as_deref(), - skills, - all, - yes, - ctx.output_format, - ctx.compact, - ) - .await + commands::skills::remove(&client, skills, all, yes, ctx.output_format, ctx.compact) + .await } SkillCommands::Validate { path, strict } => { let client = ctx.get_client(); @@ -3115,15 +3047,8 @@ mod tests { .expect("skills list should parse"); match list.command { Commands::Skills { - action: - SkillCommands::List { - node_limit, - root_uri, - }, - } => { - assert!(root_uri.is_none()); - assert_eq!(node_limit, 25); - } + action: SkillCommands::List { node_limit }, + } => assert_eq!(node_limit, 25), _ => panic!("expected skills list"), } @@ -3220,7 +3145,6 @@ mod tests { Commands::Skills { action: SkillCommands::Show { - root_uri, level, files, source, @@ -3228,7 +3152,6 @@ mod tests { .. }, } => { - assert!(root_uri.is_none()); assert_eq!(level, Some(2)); assert!(files); assert!(source); @@ -3237,25 +3160,6 @@ mod tests { _ => panic!("expected skills show"), } - let show_with_root_uri = Cli::try_parse_from([ - "ov", - "skills", - "show", - "code-review", - "--root-uri", - "viking://user/default/peers/alice/skills", - ]) - .expect("skills show root_uri should parse"); - match show_with_root_uri.command { - Commands::Skills { - action: SkillCommands::Show { root_uri, .. }, - } => assert_eq!( - root_uri.as_deref(), - Some("viking://user/default/peers/alice/skills") - ), - _ => panic!("expected skills show"), - } - let show_global_output = Cli::try_parse_from(["ov", "skills", "show", "code-review", "-o", "json"]) .expect("skills show should accept global -o after the subcommand"); diff --git a/openviking/async_client.py b/openviking/async_client.py index a26a9aca14..3312475832 100644 --- a/openviking/async_client.py +++ b/openviking/async_client.py @@ -336,9 +336,6 @@ async def summarize(self, resource_uris: Union[str, List[str]], **kwargs) -> Dic async def add_skill( self, data: Any, - to: Optional[str] = None, - parent: Optional[str] = None, - create_parent: bool = False, wait: bool = False, timeout: float = None, telemetry: TelemetryRequest = False, @@ -352,9 +349,6 @@ async def add_skill( await self._ensure_initialized() return await self._client.add_skill( data=data, - to=to, - parent=parent, - create_parent=create_parent, wait=wait, timeout=timeout, telemetry=telemetry, diff --git a/openviking/client/local.py b/openviking/client/local.py index f9a223123d..87c09901eb 100644 --- a/openviking/client/local.py +++ b/openviking/client/local.py @@ -135,9 +135,6 @@ async def add_resource( async def add_skill( self, data: Any, - to: Optional[str] = None, - parent: Optional[str] = None, - create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, telemetry: TelemetryRequest = False, @@ -149,9 +146,6 @@ async def add_skill( fn=lambda: self._service.resources.add_skill( data=data, ctx=self._ctx, - to=to, - parent=parent, - create_parent=create_parent, wait=wait, timeout=timeout, ), diff --git a/openviking/core/namespace.py b/openviking/core/namespace.py index 1981d1aa0f..aa75f2d4b7 100644 --- a/openviking/core/namespace.py +++ b/openviking/core/namespace.py @@ -13,6 +13,7 @@ _CONTENT_TYPES_BY_SCOPE = { "user": {"memories": "memory", "resources": "resource", "skills": "skill"}, } +_PEER_CONTENT_SEGMENTS = frozenset({"memories", "resources"}) _USER_RELATIVE_ROOT_SEGMENTS = frozenset({"peers", "privacy", "sessions"}) _CONTENT_SEGMENT_BY_KIND = {"resource": "resources", "skill": "skills"} @@ -117,9 +118,9 @@ def _content_segment_index(parts: tuple[str, ...]) -> Optional[int]: return None if parts[1] in _CONTENT_TYPES_BY_SCOPE["user"]: return 1 - if len(parts) >= 4 and parts[1] == "peers" and parts[3] in _CONTENT_TYPES_BY_SCOPE["user"]: + if len(parts) >= 4 and parts[1] == "peers" and parts[3] in _PEER_CONTENT_SEGMENTS: return 3 - if len(parts) >= 5 and parts[2] == "peers" and parts[4] in _CONTENT_TYPES_BY_SCOPE["user"]: + if len(parts) >= 5 and parts[2] == "peers" and parts[4] in _PEER_CONTENT_SEGMENTS: return 4 if len(parts) >= 3 and parts[2] in _CONTENT_TYPES_BY_SCOPE["user"]: return 2 diff --git a/openviking/core/peer_id.py b/openviking/core/peer_id.py index c1cdf1145a..4d3782770d 100644 --- a/openviking/core/peer_id.py +++ b/openviking/core/peer_id.py @@ -15,11 +15,3 @@ def normalize_peer_id( return normalize_identifier_part(peer_id, "peer_id") except ValueError as exc: raise ValueError(f"Invalid peer_id: {exc}") from exc - - -def safe_peer_id(peer_id: Optional[str]) -> Optional[str]: - """Return a usable peer_id, or None for empty/path-like values.""" - try: - return normalize_peer_id(peer_id) - except ValueError: - return None diff --git a/openviking/core/retrieval_targets.py b/openviking/core/retrieval_targets.py index 2c09877bcf..1b5ad5f2e0 100644 --- a/openviking/core/retrieval_targets.py +++ b/openviking/core/retrieval_targets.py @@ -66,11 +66,11 @@ def default_target_directories( if context_type == ContextType.RESOURCE: return _default_resource_targets(ctx, peer_id) if context_type == ContextType.SKILL: - return _default_user_scoped_targets(ctx, peer_id, "skills") + return _default_skill_targets(ctx) return [ *_default_user_scoped_targets(ctx, peer_id, "memories"), *_default_resource_targets(ctx, peer_id), - *_default_user_scoped_targets(ctx, peer_id, "skills"), + *_default_skill_targets(ctx), ] @@ -114,6 +114,8 @@ def _target_directories_for_uri( for segment in ("memories", "resources", "skills"): if _is_default_user_content_root(target_uri, ctx, segment): + if segment == "skills": + return _default_skill_targets(ctx) return _default_user_scoped_targets(ctx, peer_id, segment) return [target_uri] @@ -123,7 +125,7 @@ def _default_user_root_targets(ctx: RequestContext, peer_id: Optional[str]) -> L return [ *_default_user_scoped_targets(ctx, peer_id, "memories"), *_default_user_scoped_targets(ctx, peer_id, "resources"), - *_default_user_scoped_targets(ctx, peer_id, "skills"), + *_default_skill_targets(ctx), ] @@ -134,6 +136,10 @@ def _default_resource_targets(ctx: RequestContext, peer_id: Optional[str]) -> Li ] +def _default_skill_targets(ctx: RequestContext) -> List[str]: + return [f"{canonical_user_root(ctx)}/skills"] + + def _default_user_scoped_targets( ctx: RequestContext, peer_id: Optional[str], @@ -172,10 +178,9 @@ def _resolve_peer_target( return [ f"{peer_root}/memories", f"{peer_root}/resources", - f"{peer_root}/skills", ] - if suffix[2] not in {"memories", "resources", "skills"}: - raise InvalidArgumentError("Only peer memories, resources, and skills are searchable.") + if suffix[2] not in {"memories", "resources"}: + raise InvalidArgumentError("Only peer memories and resources are searchable.") return [target_uri] diff --git a/openviking/server/routers/resources.py b/openviking/server/routers/resources.py index 821d61e7f0..dfe6028dd6 100644 --- a/openviking/server/routers/resources.py +++ b/openviking/server/routers/resources.py @@ -97,9 +97,6 @@ class AddSkillRequest(BaseModel): data: Inline skill content or structured skill data. HTTP requests do not treat string values as host filesystem paths. temp_file_id: Temporary upload id returned by /api/v1/resources/temp_upload. - to: Exact target URI for the skill. - parent: Parent skills URI under which the skill name will be stored. - create_parent: Whether to automatically create parent when it does not exist. wait: Whether to wait for skill processing to complete. timeout: Timeout in seconds when wait=True. """ @@ -108,9 +105,6 @@ class AddSkillRequest(BaseModel): data: Any = None temp_file_id: Optional[str] = None - to: Optional[str] = None - parent: Optional[str] = None - create_parent: bool = False wait: bool = False timeout: Optional[float] = None source_metadata: Optional[Dict[str, Any]] = None @@ -308,9 +302,6 @@ async def _add() -> dict[str, Any]: result = await service.resources.add_skill( data=data, ctx=_ctx, - to=request.to, - parent=request.parent, - create_parent=request.create_parent, wait=request.wait, timeout=request.timeout, allow_local_path_resolution=allow_local_path_resolution, diff --git a/openviking/server/routers/skills.py b/openviking/server/routers/skills.py index fb6db64898..d6e981dfb4 100644 --- a/openviking/server/routers/skills.py +++ b/openviking/server/routers/skills.py @@ -13,12 +13,8 @@ from fastapi import Path as ApiPath from pydantic import BaseModel, ConfigDict, model_validator -from openviking.core.namespace import ( - canonical_user_content_root, - is_content_root_uri, -) +from openviking.core.namespace import canonical_user_root from openviking.core.skill_loader import SkillLoader -from openviking.core.uri_validation import validate_content_target_uri from openviking.privacy.service import UserPrivacyConfigVersion from openviking.server.auth import get_request_context from openviking.server.dependencies import get_service @@ -45,7 +41,6 @@ class UpdateSkillRequest(BaseModel): data: Any = None temp_file_id: Optional[str] = None - root_uri: Optional[str] = None wait: bool = False timeout: Optional[float] = None source_metadata: Optional[Dict[str, Any]] = None @@ -62,7 +57,6 @@ class FindSkillsRequest(BaseModel): model_config = ConfigDict(extra="forbid") query: str - root_uri: Optional[str] = None limit: int = 10 score_threshold: Optional[float] = None level: Optional[list[int]] = None @@ -79,24 +73,15 @@ class ValidateSkillRequest(BaseModel): def _agent_skills_root(ctx: RequestContext) -> str: - return canonical_user_content_root(ctx, "skill") - - -def _resolve_skills_root(ctx: RequestContext, root_uri: Optional[str]) -> str: - if not root_uri: - return _agent_skills_root(ctx) - resolved = validate_content_target_uri(root_uri, ctx, kind="skill", field_name="root_uri") - if not is_content_root_uri(resolved, ctx, kind="skill"): - raise InvalidArgumentError("root_uri must point to a skills root.") - return resolved.rstrip("/") + return f"{canonical_user_root(ctx)}/skills" def _validate_skill_name(skill_name: str) -> str: return validate_skill_name(skill_name) -def _skill_root_uri(skills_root_uri: str, skill_name: str) -> str: - return f"{skills_root_uri.rstrip('/')}/{_validate_skill_name(skill_name)}" +def _skill_root_uri(ctx: RequestContext, skill_name: str) -> str: + return f"{_agent_skills_root(ctx)}/{_validate_skill_name(skill_name)}" def _skill_md_uri(root_uri: str) -> str: @@ -304,13 +289,8 @@ def _skill_summary_from_hit(hit: Dict[str, Any]) -> Dict[str, Any]: return summary -async def _require_skill( - service, - ctx: RequestContext, - skill_name: str, - skills_root_uri: str, -) -> str: - root_uri = _skill_root_uri(skills_root_uri, skill_name) +async def _require_skill(service, ctx: RequestContext, skill_name: str) -> str: + root_uri = _skill_root_uri(ctx, skill_name) try: stat = await service.fs.stat(root_uri, ctx=ctx) except NotFoundError: @@ -431,12 +411,11 @@ async def _restore_skill_privacy( @router.get("") async def list_skills( node_limit: int = 1000, - root_uri: Optional[str] = None, _ctx: RequestContext = Depends(get_request_context), ): """List installed agent skills.""" service = get_service() - root_uri = _resolve_skills_root(_ctx, root_uri) + root_uri = _agent_skills_root(_ctx) try: entries = await service.fs.ls( root_uri, @@ -464,7 +443,7 @@ async def find_skills( ): """Find agent skills by semantic search.""" service = get_service() - root_uri = _resolve_skills_root(_ctx, request.root_uri) + root_uri = _agent_skills_root(_ctx) execution = await run_operation( operation="skills.find", telemetry=request.telemetry, @@ -512,7 +491,6 @@ async def get_skill( include_files: bool = True, include_source: bool = False, level: Optional[int] = None, - root_uri: Optional[str] = None, _ctx: RequestContext = Depends(get_request_context), ): """Show one installed agent skill.""" @@ -522,36 +500,34 @@ async def get_skill( details={"field": "level", "allowed": [0, 1, 2]}, ) service = get_service() - skills_root_uri = _resolve_skills_root(_ctx, root_uri) - skill_root_uri = await _require_skill(service, _ctx, skill_name, skills_root_uri) - abstract = await service.fs.abstract(skill_root_uri, ctx=_ctx) - result = _skill_summary_from_meta(skill_name, skill_root_uri, _parse_abstract_meta(abstract)) + root_uri = await _require_skill(service, _ctx, skill_name) + abstract = await service.fs.abstract(root_uri, ctx=_ctx) + result = _skill_summary_from_meta(skill_name, root_uri, _parse_abstract_meta(abstract)) if level is None or level == 0: result["abstract"] = abstract if level is None or level == 1: - result["overview"] = await service.fs.overview(skill_root_uri, ctx=_ctx) + result["overview"] = await service.fs.overview(root_uri, ctx=_ctx) if level == 2 or include_content is True or (level is None and include_content is not False): - result["content"] = await service.fs.read(_skill_md_uri(skill_root_uri), ctx=_ctx) + result["content"] = await service.fs.read(_skill_md_uri(root_uri), ctx=_ctx) if include_files: - entries = await _list_skill_files(service, _ctx, skill_root_uri) + entries = await _list_skill_files(service, _ctx, root_uri) result["files"] = [ { "name": entry.get("name") or _skill_name_from_uri(entry.get("uri", "")), "uri": entry.get("uri", ""), - "path": _relative_skill_path(skill_root_uri, entry.get("uri", "")), + "path": _relative_skill_path(root_uri, entry.get("uri", "")), "is_dir": entry.get("isDir", False), "kind": _skill_file_kind( - _relative_skill_path(skill_root_uri, entry.get("uri", "")), + _relative_skill_path(root_uri, entry.get("uri", "")), entry.get("isDir", False), ), } for entry in entries if isinstance(entry, dict) - and _relative_skill_path(skill_root_uri, entry.get("uri", "")) - != SOURCE_METADATA_FILENAME + and _relative_skill_path(root_uri, entry.get("uri", "")) != SOURCE_METADATA_FILENAME ] if include_source: - result["source"] = await read_skill_source_metadata(service, _ctx, skill_root_uri) + result["source"] = await read_skill_source_metadata(service, _ctx, root_uri) return Response(status="ok", result=result) @@ -564,8 +540,7 @@ async def update_skill( ): """Replace an existing agent skill with new content.""" service = get_service() - skills_root_uri = _resolve_skills_root(_ctx, request.root_uri) - skill_root_uri = await _require_skill(service, _ctx, skill_name, skills_root_uri) + root_uri = await _require_skill(service, _ctx, skill_name) data = request.data allow_local_path_resolution = False @@ -594,7 +569,7 @@ async def update_skill( store = TempUploadStore.build(http_request.app.state.config) if resolved else None async def _update() -> Dict[str, Any]: - backup_uri = f"{skills_root_uri}/.{skill_name}.update-backup-{uuid.uuid4().hex}" + backup_uri = f"{_agent_skills_root(_ctx)}/.{skill_name}.update-backup-{uuid.uuid4().hex}" backup_created = False privacy_update_attempted = False previous_privacy = None @@ -618,12 +593,11 @@ async def _update() -> Dict[str, Any]: "actual": preparation.skill_dict.get("name"), }, ) - await service.fs.mv(skill_root_uri, backup_uri, ctx=_ctx) + await service.fs.mv(root_uri, backup_uri, ctx=_ctx) backup_created = True result = await service.resources.add_skill( data=preparation, ctx=_ctx, - to=skill_root_uri, wait=request.wait, timeout=request.timeout, allow_local_path_resolution=False, @@ -643,11 +617,11 @@ async def _update() -> Dict[str, Any]: except Exception: if backup_created: try: - await service.fs.rm(skill_root_uri, ctx=_ctx, recursive=True) + await service.fs.rm(root_uri, ctx=_ctx, recursive=True) except Exception: pass try: - await service.fs.mv(backup_uri, skill_root_uri, ctx=_ctx) + await service.fs.mv(backup_uri, root_uri, ctx=_ctx) except Exception: pass if privacy_update_attempted: @@ -686,22 +660,20 @@ async def _update() -> Dict[str, Any]: @router.delete("/{skill_name}") async def delete_skill( skill_name: str = ApiPath(..., description="Skill name"), - root_uri: Optional[str] = None, _ctx: RequestContext = Depends(get_request_context), ): """Remove one installed agent skill.""" service = get_service() - skills_root_uri = _resolve_skills_root(_ctx, root_uri) - skill_root_uri = await _require_skill(service, _ctx, skill_name, skills_root_uri) - result = await service.fs.rm(skill_root_uri, ctx=_ctx, recursive=True) + root_uri = await _require_skill(service, _ctx, skill_name) + result = await service.fs.rm(root_uri, ctx=_ctx, recursive=True) privacy_deleted = False privacy = service.privacy_configs if privacy is not None: privacy_deleted = await privacy.delete(_ctx, "skill", skill_name) response_result: Dict[str, Any] = { "name": skill_name, - "uri": skill_root_uri, - "root_uri": skill_root_uri, + "uri": root_uri, + "root_uri": root_uri, } if isinstance(result, dict) and "estimated_deleted_count" in result: response_result["estimated_deleted_count"] = result["estimated_deleted_count"] diff --git a/openviking/service/resource_service.py b/openviking/service/resource_service.py index 596e9aeca5..f7401c45fb 100644 --- a/openviking/service/resource_service.py +++ b/openviking/service/resource_service.py @@ -806,9 +806,6 @@ async def add_skill( self, data: Any, ctx: RequestContext, - to: Optional[str] = None, - parent: Optional[str] = None, - create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, allow_local_path_resolution: bool = True, @@ -834,19 +831,11 @@ async def add_skill( request_wait_tracker.register_request(telemetry_id) try: - target = ContentTargetSpec.from_fields( - ctx=ctx, - kind="skill", - to=to, - parent=parent, - create_parent=create_parent, - ) if isinstance(data, SkillProcessingPreparation): result = await self._skill_processor.process_prepared_skill( data, viking_fs=self._viking_fs, ctx=ctx, - target=target, apply_privacy=apply_privacy, privacy_change_reason=privacy_change_reason, ) @@ -855,7 +844,6 @@ async def add_skill( data=data, viking_fs=self._viking_fs, ctx=ctx, - target=target, allow_local_path_resolution=allow_local_path_resolution, source_path_hint=source_path_hint, apply_privacy=apply_privacy, diff --git a/openviking/session/memory/memory_isolation_handler.py b/openviking/session/memory/memory_isolation_handler.py index b8fbf5284f..2b5ac81a35 100644 --- a/openviking/session/memory/memory_isolation_handler.py +++ b/openviking/session/memory/memory_isolation_handler.py @@ -5,7 +5,6 @@ from dataclasses import dataclass, field from typing import Any, Dict, List, Optional, Set -from openviking.core.peer_id import safe_peer_id from openviking.server.identity import RequestContext from openviking.session.memory.dataclass import MemoryTypeSchema, ResolvedOperation from openviking.session.memory.memory_updater import ExtractContext @@ -48,11 +47,9 @@ def __init__( if allowed_memory_types is not None else None ) - peer_ids = {safe_peer_id(item) for item in allowed_peer_ids or set()} - peer_ids = {item for item in peer_ids if item} self.allow_self = bool(allow_self) - self.allowed_peer_ids = peer_ids - self.allow_peer = bool(peer_ids) + self.allowed_peer_ids = {item for item in allowed_peer_ids or set() if item} + self.allow_peer = bool(self.allowed_peer_ids) def prepare_messages(self) -> None: """No-op hook kept for the extraction pipeline.""" @@ -66,13 +63,13 @@ def _has_self_user_message(self) -> bool: for msg in self._messages(): if getattr(msg, "role", None) != "user": continue - if safe_peer_id(getattr(msg, "peer_id", None)) is None: + if not getattr(msg, "peer_id", None): return True return False def _first_peer_id_in_messages(self) -> Optional[str]: for msg in self._messages(): - peer_id = safe_peer_id(getattr(msg, "peer_id", None)) + peer_id = getattr(msg, "peer_id", None) if peer_id and self._can_write_peer(peer_id): return peer_id return None @@ -100,7 +97,7 @@ def fill_identity_fields(self, item_dict: Dict[str, Any], role_scope: RoleScope) item_dict["user_id"] = self.ctx.user.user_id item_dict.pop("user_ids", None) - peer_id = safe_peer_id(item_dict.get("peer_id")) + peer_id = item_dict.get("peer_id") if peer_id: item_dict["peer_id"] = peer_id else: @@ -151,8 +148,7 @@ def _range_targets(self, ranges: Any) -> tuple[bool, List[str]]: peer_ids = set() for msg_group in getattr(msg_range, "elements", []) or []: for msg in msg_group: - raw_peer_id = getattr(msg, "peer_id", None) - peer_id = safe_peer_id(raw_peer_id) + peer_id = getattr(msg, "peer_id", None) if peer_id: if self._can_write_peer(peer_id): peer_ids.add(peer_id) @@ -184,10 +180,7 @@ def calculate_memory_uris( ) operation.memory_fields.pop("peer_id", None) else: - raw_peer_id = operation.memory_fields.get("peer_id") - peer_id = safe_peer_id(raw_peer_id) - if raw_peer_id and not peer_id: - return [] + peer_id = operation.memory_fields.get("peer_id") if peer_id: if not self._can_write_peer(peer_id): return [] diff --git a/openviking/session/session.py b/openviking/session/session.py index 4b08dc6756..2fc980ca00 100644 --- a/openviking/session/session.py +++ b/openviking/session/session.py @@ -14,7 +14,7 @@ from uuid import uuid4 from openviking.core.namespace import canonical_session_uri -from openviking.core.peer_id import normalize_peer_id, safe_peer_id +from openviking.core.peer_id import normalize_peer_id from openviking.message import Message, Part from openviking.message.part import ContextPart, TextPart, ToolPart from openviking.server.config import ToolOutputExternalizationConfig @@ -86,7 +86,7 @@ def _message_peer_ids(messages: List[Message]) -> set[str]: return { peer_id for message in messages - if (peer_id := safe_peer_id(getattr(message, "peer_id", None))) + if (peer_id := getattr(message, "peer_id", None)) } diff --git a/openviking/sync_client.py b/openviking/sync_client.py index e7359335f8..5b01d22635 100644 --- a/openviking/sync_client.py +++ b/openviking/sync_client.py @@ -210,9 +210,6 @@ def add_resource( def add_skill( self, data: Any, - to: Optional[str] = None, - parent: Optional[str] = None, - create_parent: bool = False, wait: bool = False, timeout: float = None, telemetry: TelemetryRequest = False, @@ -221,9 +218,6 @@ def add_skill( return run_async( self._async_client.add_skill( data, - to=to, - parent=parent, - create_parent=create_parent, wait=wait, timeout=timeout, telemetry=telemetry, diff --git a/openviking/utils/skill_processor.py b/openviking/utils/skill_processor.py index c95b129ee5..4bb811df1b 100644 --- a/openviking/utils/skill_processor.py +++ b/openviking/utils/skill_processor.py @@ -17,12 +17,10 @@ import yaml -from openviking.core.content_targets import ContentTargetSpec from openviking.core.context import Context, ContextType, Vectorize from openviking.core.mcp_converter import is_mcp_format, mcp_to_skill from openviking.core.namespace import ( - canonical_user_content_root, - uri_leaf_name, + canonical_user_root, user_space_fragment, ) from openviking.core.skill_loader import SkillLoader @@ -41,7 +39,6 @@ from openviking_cli.exceptions import InvalidArgumentError from openviking_cli.utils import get_logger from openviking_cli.utils.config import get_openviking_config -from openviking_cli.utils.uri import VikingURI logger = get_logger(__name__) @@ -119,7 +116,6 @@ async def process_skill( data: Any, viking_fs: VikingFS, ctx: RequestContext, - target: Optional[ContentTargetSpec] = None, allow_local_path_resolution: bool = True, source_path_hint: Optional[str] = None, apply_privacy: bool = True, @@ -155,7 +151,6 @@ async def process_skill( preparation, viking_fs=viking_fs, ctx=ctx, - target=target, apply_privacy=apply_privacy, privacy_change_reason=privacy_change_reason, ) @@ -166,7 +161,6 @@ async def process_prepared_skill( viking_fs: VikingFS, ctx: RequestContext, *, - target: Optional[ContentTargetSpec] = None, apply_privacy: bool = True, privacy_change_reason: str = "auto-extracted from add_skill", ) -> Dict[str, Any]: @@ -187,14 +181,9 @@ async def process_prepared_skill( ) skill_abstract = self._build_skill_abstract(skill_dict) - skill_dir_uri, skill_root_uri = await self._resolve_skill_target_uri( - viking_fs=viking_fs, - ctx=ctx, - skill_name=skill_dict["name"], - target=target, - ) + skill_root_uri = f"{canonical_user_root(ctx)}/skills" context = Context( - uri=skill_dir_uri, + uri=f"{skill_root_uri}/{skill_dict['name']}", parent_uri=skill_root_uri, is_leaf=False, abstract=skill_abstract, @@ -219,6 +208,8 @@ async def process_prepared_skill( round((time.perf_counter() - overview_start) * 1000, 3), ) + skill_dir_uri = context.uri + write_start = time.perf_counter() await self._write_skill_content( viking_fs=viking_fs, @@ -286,40 +277,6 @@ async def prepare_skill_processing( shutil.rmtree(cleanup_path, ignore_errors=True) raise - async def _resolve_skill_target_uri( - self, - *, - viking_fs: VikingFS, - ctx: RequestContext, - skill_name: str, - target: Optional[ContentTargetSpec], - ) -> tuple[str, str]: - target = target or ContentTargetSpec() - if target.to: - target_uri = target.to.rstrip("/") - target_name = uri_leaf_name(target_uri) - if target_name != skill_name: - raise InvalidArgumentError( - f"Skill target URI name mismatch: target name is '{target_name}', " - f"content name is '{skill_name}'", - details={"expected": skill_name, "actual": target_name}, - ) - parent_uri_obj = VikingURI(target_uri).parent - if parent_uri_obj is None: - raise InvalidArgumentError( - f"Skill target URI must be under a skills root: {target_uri}" - ) - return target_uri, parent_uri_obj.uri - - skill_root_uri = (target.parent or canonical_user_content_root(ctx, "skill")).rstrip("/") - if target.parent: - await viking_fs.ensure_parent_exists( - skill_root_uri, - ctx, - create_parent=target.create_parent, - ) - return f"{skill_root_uri}/{skill_name}", skill_root_uri - def _parse_skill( self, data: Any, diff --git a/openviking_cli/client/base.py b/openviking_cli/client/base.py index 490587f1a5..066f12b6d1 100644 --- a/openviking_cli/client/base.py +++ b/openviking_cli/client/base.py @@ -51,9 +51,6 @@ async def add_resource( async def add_skill( self, data: Any, - to: Optional[str] = None, - parent: Optional[str] = None, - create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, telemetry: TelemetryRequest = False, diff --git a/openviking_cli/client/http.py b/openviking_cli/client/http.py index 3e3ab04c8c..cb84fd31e0 100644 --- a/openviking_cli/client/http.py +++ b/openviking_cli/client/http.py @@ -477,9 +477,6 @@ async def batch_add_messages( async def add_skill( self, data: Any, - to: Optional[str] = None, - parent: Optional[str] = None, - create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, telemetry: TelemetryRequest = False, @@ -487,9 +484,6 @@ async def add_skill( """Add skill to OpenViking.""" telemetry = self._validate_telemetry(telemetry) request_data = { - "to": to, - "parent": parent, - "create_parent": create_parent, "wait": wait, "timeout": timeout, } diff --git a/openviking_cli/client/sync_http.py b/openviking_cli/client/sync_http.py index 064dcce0b0..5eab6ccbe6 100644 --- a/openviking_cli/client/sync_http.py +++ b/openviking_cli/client/sync_http.py @@ -260,9 +260,6 @@ def add_resource( def add_skill( self, data: Any, - to: Optional[str] = None, - parent: Optional[str] = None, - create_parent: bool = False, wait: bool = False, timeout: Optional[float] = None, telemetry: TelemetryRequest = False, @@ -271,9 +268,6 @@ def add_skill( return run_async( self._async_client.add_skill( data, - to=to, - parent=parent, - create_parent=create_parent, wait=wait, timeout=timeout, telemetry=telemetry, diff --git a/tests/server/test_api_skills.py b/tests/server/test_api_skills.py index 7c0067cfd8..83fac085d7 100644 --- a/tests/server/test_api_skills.py +++ b/tests/server/test_api_skills.py @@ -169,71 +169,6 @@ async def test_skills_api_list_show_find_and_delete(client): assert missing_privacy_response.status_code == 404 -async def test_skills_api_manages_peer_skills_with_root_uri(client): - peer_root_uri = "viking://user/default/peers/alice/skills" - add_response = await client.post( - "/api/v1/skills", - json={ - "data": _skill_md("peer-skill", "Peer skill"), - "to": f"{peer_root_uri}/peer-skill", - "wait": True, - }, - ) - assert add_response.status_code == 200, add_response.text - assert add_response.json()["result"]["root_uri"] == f"{peer_root_uri}/peer-skill" - - default_list_response = await client.get("/api/v1/skills") - assert default_list_response.status_code == 200, default_list_response.text - assert all( - skill["name"] != "peer-skill" - for skill in default_list_response.json()["result"]["skills"] - ) - - peer_list_response = await client.get( - "/api/v1/skills", - params={"root_uri": peer_root_uri}, - ) - assert peer_list_response.status_code == 200, peer_list_response.text - assert any( - skill["name"] == "peer-skill" - for skill in peer_list_response.json()["result"]["skills"] - ) - - show_response = await client.get( - "/api/v1/skills/peer-skill", - params={"root_uri": peer_root_uri, "level": 2}, - ) - assert show_response.status_code == 200, show_response.text - assert show_response.json()["result"]["root_uri"] == f"{peer_root_uri}/peer-skill" - - find_response = await client.post( - "/api/v1/skills/find", - json={"query": "peer", "root_uri": peer_root_uri, "limit": 5}, - ) - assert find_response.status_code == 200, find_response.text - assert find_response.json()["result"]["root_uri"] == peer_root_uri - - delete_response = await client.delete( - "/api/v1/skills/peer-skill", - params={"root_uri": peer_root_uri}, - ) - assert delete_response.status_code == 200, delete_response.text - assert delete_response.json()["result"]["root_uri"] == f"{peer_root_uri}/peer-skill" - - -async def test_skills_api_rejects_skill_target_under_resources(client): - response = await client.post( - "/api/v1/skills", - json={ - "data": _skill_md("wrong-root-skill", "Wrong root skill"), - "parent": "viking://user/default/peers/alice/resources", - "wait": True, - }, - ) - assert response.status_code == 400, response.text - assert response.json()["error"]["code"] == "INVALID_URI" - - async def test_skills_api_update_requires_matching_name(client): await _add_skill(client, "update-skill", "Original description") diff --git a/tests/server/test_peer_id_compat.py b/tests/server/test_peer_id_compat.py index 9887590bf0..b667f74a52 100644 --- a/tests/server/test_peer_id_compat.py +++ b/tests/server/test_peer_id_compat.py @@ -17,9 +17,9 @@ def test_normalize_peer_id_accepts_peer_id(): assert normalize_peer_id("web-visitor-alice") == "web-visitor-alice" -def test_normalize_peer_id_rejects_colon(): +def test_normalize_peer_id_rejects_invalid_character(): with pytest.raises(ValueError, match="Invalid peer_id"): - normalize_peer_id("web:visitor:alice") + normalize_peer_id("web+visitor+alice") def _target_dirs(target_uri="", peer_id=None): @@ -48,7 +48,6 @@ def test_default_peer_search_targets_self_and_requested_peer_content(): "viking://user/support_bot/resources", "viking://user/support_bot/peers/web-visitor-alice/resources", "viking://user/support_bot/skills", - "viking://user/support_bot/peers/web-visitor-alice/skills", ] @@ -73,7 +72,6 @@ def test_peer_search_explicit_peer_root_targets_that_peer_content(): assert targets == [ "viking://user/support_bot/peers/web-visitor-alice/memories", "viking://user/support_bot/peers/web-visitor-alice/resources", - "viking://user/support_bot/peers/web-visitor-alice/skills", ] @@ -96,7 +94,7 @@ def test_peer_search_explicit_peer_target_rejects_mismatched_peer_id(): def test_peer_search_explicit_peer_target_rejects_invalid_peer_id(): with pytest.raises(InvalidArgumentError, match="Invalid peer_id"): - _target_dirs("viking://user/support_bot/peers/web:visitor:alice/memories") + _target_dirs("viking://user/support_bot/peers/web+visitor+alice/memories") def test_peer_search_rejects_all_peers_target(): @@ -113,7 +111,6 @@ def test_peer_search_user_root_targets_user_content_and_requested_peer_content() "viking://user/support_bot/resources", "viking://user/support_bot/peers/web-visitor-alice/resources", "viking://user/support_bot/skills", - "viking://user/support_bot/peers/web-visitor-alice/skills", ] diff --git a/tests/unit/test_namespace_uri_classification.py b/tests/unit/test_namespace_uri_classification.py index 27eda8a12d..512f93f527 100644 --- a/tests/unit/test_namespace_uri_classification.py +++ b/tests/unit/test_namespace_uri_classification.py @@ -37,12 +37,6 @@ def test_context_type_for_uri_uses_path_segments(): context_type_for_uri("viking://user/support_bot/peers/web-visitor-alice/resources/faq.md") == "resource" ) - assert ( - context_type_for_uri( - "viking://user/support_bot/peers/web-visitor-alice/skills/search-helper" - ) - == "skill" - ) assert context_type_for_uri("viking://resources/memories-report.md") == "resource" assert context_type_for_uri("viking://user/alice/resources/skills-report.md") == "resource" @@ -55,9 +49,9 @@ def test_peer_content_target_uri_rejects_invalid_peer_id(): with pytest.raises(InvalidURIError, match="Invalid peer_id"): validate_content_target_uri( - "viking://user/support_bot/peers/web:visitor:alice/skills/demo", + "viking://user/support_bot/peers/web+visitor+alice/resources/demo", ctx, - kind="skill", + kind="resource", ) From dfdb53f8150b6a629e4c573658e55884eb1589da Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Fri, 12 Jun 2026 12:03:01 +0800 Subject: [PATCH 4/7] chore: drop incidental formatting changes --- crates/ov_cli/src/commands/skills.rs | 16 +++------------- crates/ov_cli/src/main.rs | 5 +---- openviking/server/routers/resources.py | 1 - openviking/server/routers/skills.py | 6 +----- openviking/sync_client.py | 7 +------ openviking/utils/skill_processor.py | 5 +---- openviking_cli/client/sync_http.py | 7 +------ 7 files changed, 8 insertions(+), 39 deletions(-) diff --git a/crates/ov_cli/src/commands/skills.rs b/crates/ov_cli/src/commands/skills.rs index c1685b6673..855a82c598 100644 --- a/crates/ov_cli/src/commands/skills.rs +++ b/crates/ov_cli/src/commands/skills.rs @@ -800,9 +800,7 @@ fn normalize_git_subdir(subdir: &Path) -> Result { } if normalized.as_os_str().is_empty() { - return Err(Error::Parse( - "Git source metadata missing subdir".to_string(), - )); + return Err(Error::Parse("Git source metadata missing subdir".to_string())); } Ok(normalized) @@ -1938,11 +1936,7 @@ mod tests { let error = update_target_from_record(&record, "local-skill", false) .expect_err("local source should not be trusted for non-interactive update"); - assert!( - error - .to_string() - .contains("server-recorded local paths are not trusted") - ); + assert!(error.to_string().contains("server-recorded local paths are not trusted")); } #[test] @@ -1960,11 +1954,7 @@ mod tests { let error = prepare_source_from_git_record(&record) .err() .expect("parent dir subdir should be rejected before clone"); - assert!( - error - .to_string() - .contains("parent directory components are not allowed") - ); + assert!(error.to_string().contains("parent directory components are not allowed")); } #[test] diff --git a/crates/ov_cli/src/main.rs b/crates/ov_cli/src/main.rs index 4f77b86ced..9e01b0413a 100644 --- a/crates/ov_cli/src/main.rs +++ b/crates/ov_cli/src/main.rs @@ -3169,10 +3169,7 @@ mod tests { .expect("skills remove --yes should parse"); match remove.command { Commands::Skills { - action: - SkillCommands::Remove { - skills, yes, all, .. - }, + action: SkillCommands::Remove { skills, yes, all }, } => { assert_eq!(skills, vec!["foo", "bar"]); assert!(yes); diff --git a/openviking/server/routers/resources.py b/openviking/server/routers/resources.py index dfe6028dd6..d79f8d6078 100644 --- a/openviking/server/routers/resources.py +++ b/openviking/server/routers/resources.py @@ -270,7 +270,6 @@ async def add_skill( ): """Add skill to OpenViking.""" service = get_service() - data = request.data allow_local_path_resolution = False resolved = None diff --git a/openviking/server/routers/skills.py b/openviking/server/routers/skills.py index d6e981dfb4..9f89ea9c4e 100644 --- a/openviking/server/routers/skills.py +++ b/openviking/server/routers/skills.py @@ -670,11 +670,7 @@ async def delete_skill( privacy = service.privacy_configs if privacy is not None: privacy_deleted = await privacy.delete(_ctx, "skill", skill_name) - response_result: Dict[str, Any] = { - "name": skill_name, - "uri": root_uri, - "root_uri": root_uri, - } + response_result: Dict[str, Any] = {"name": skill_name, "uri": root_uri, "root_uri": root_uri} if isinstance(result, dict) and "estimated_deleted_count" in result: response_result["estimated_deleted_count"] = result["estimated_deleted_count"] response_result["privacy_deleted"] = privacy_deleted diff --git a/openviking/sync_client.py b/openviking/sync_client.py index 5b01d22635..6ecc91726a 100644 --- a/openviking/sync_client.py +++ b/openviking/sync_client.py @@ -216,12 +216,7 @@ def add_skill( ) -> Dict[str, Any]: """Add skill to OpenViking.""" return run_async( - self._async_client.add_skill( - data, - wait=wait, - timeout=timeout, - telemetry=telemetry, - ) + self._async_client.add_skill(data, wait=wait, timeout=timeout, telemetry=telemetry) ) def search( diff --git a/openviking/utils/skill_processor.py b/openviking/utils/skill_processor.py index 4bb811df1b..dd7f1fcab2 100644 --- a/openviking/utils/skill_processor.py +++ b/openviking/utils/skill_processor.py @@ -19,10 +19,7 @@ from openviking.core.context import Context, ContextType, Vectorize from openviking.core.mcp_converter import is_mcp_format, mcp_to_skill -from openviking.core.namespace import ( - canonical_user_root, - user_space_fragment, -) +from openviking.core.namespace import canonical_user_root, user_space_fragment from openviking.core.skill_loader import SkillLoader from openviking.privacy import ( UserPrivacyConfigService, diff --git a/openviking_cli/client/sync_http.py b/openviking_cli/client/sync_http.py index 5eab6ccbe6..a624896f83 100644 --- a/openviking_cli/client/sync_http.py +++ b/openviking_cli/client/sync_http.py @@ -266,12 +266,7 @@ def add_skill( ) -> Dict[str, Any]: """Add skill to OpenViking.""" return run_async( - self._async_client.add_skill( - data, - wait=wait, - timeout=timeout, - telemetry=telemetry, - ) + self._async_client.add_skill(data, wait=wait, timeout=timeout, telemetry=telemetry) ) def wait_processed(self, timeout: Optional[float] = None) -> Dict[str, Any]: From c3d67ccccf42d392820ed25340344e2a41d1b25a Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Fri, 12 Jun 2026 12:21:35 +0800 Subject: [PATCH 5/7] fix(storage): revert shared parent existence helper --- openviking/parse/tree_builder.py | 25 +++++++++++++++++----- openviking/storage/ovpack/operations.py | 9 +++++++- openviking/storage/viking_fs.py | 28 +------------------------ 3 files changed, 29 insertions(+), 33 deletions(-) diff --git a/openviking/parse/tree_builder.py b/openviking/parse/tree_builder.py index 23aeebc5c6..5392f2fdaf 100644 --- a/openviking/parse/tree_builder.py +++ b/openviking/parse/tree_builder.py @@ -113,11 +113,26 @@ async def resolve_target_uri( effective_parent_uri = effective_parent_uri.rstrip("/") if effective_parent_uri: viking_fs = get_viking_fs() - await viking_fs.ensure_parent_exists( - effective_parent_uri, - ctx, - create_parent=create_parent, - ) + try: + parent_exists = await viking_fs.exists(effective_parent_uri, ctx=ctx) + if not parent_exists: + if create_parent: + logger.info( + f"[TreeBuilder] Parent URI does not exist, creating: {effective_parent_uri}" + ) + await viking_fs.mkdir(effective_parent_uri, exist_ok=True, ctx=ctx) + else: + raise FileNotFoundError( + f"Parent URI does not exist: {effective_parent_uri}. " + f"Use --parent-auto-create/-p to automatically create it." + ) + stat_result = await viking_fs.stat(effective_parent_uri, ctx=ctx) + except FileNotFoundError: + raise + except Exception as e: + raise FileNotFoundError(f"Parent URI does not exist: {effective_parent_uri}") from e + if not stat_result.get("isDir"): + raise ValueError(f"Parent URI is not a directory: {effective_parent_uri}") base_uri = effective_parent_uri planned_uri = VikingURI(base_uri).join(final_doc_name).uri diff --git a/openviking/storage/ovpack/operations.py b/openviking/storage/ovpack/operations.py index 4ce815cbdd..5edd2eaa30 100644 --- a/openviking/storage/ovpack/operations.py +++ b/openviking/storage/ovpack/operations.py @@ -109,6 +109,13 @@ async def _root_exists(viking_fs, root_uri: str, ctx: RequestContext) -> bool: return False +async def _ensure_parent_exists(viking_fs, parent: str, ctx: RequestContext) -> None: + try: + await viking_fs.stat(parent, ctx=ctx) + except Exception: + await viking_fs.mkdir(parent, ctx=ctx) + + async def _remove_existing_root(viking_fs, root_uri: str, ctx: RequestContext) -> None: if not hasattr(viking_fs, "rm"): logger.warning(f"[ovpack] Cannot remove existing resource without rm(): {root_uri}") @@ -325,7 +332,7 @@ async def import_ovpack( vector_mode=vector_action_mode, ) if parent != "viking://": - await viking_fs.ensure_parent_exists(parent, ctx) + await _ensure_parent_exists(viking_fs, parent, ctx) for existing_root in existing_roots: logger.info(f"[ovpack] Overwriting existing resource at {existing_root}") diff --git a/openviking/storage/viking_fs.py b/openviking/storage/viking_fs.py index 6b0f483ba7..07cd82679b 100644 --- a/openviking/storage/viking_fs.py +++ b/openviking/storage/viking_fs.py @@ -24,7 +24,7 @@ from pathlib import PurePath from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union -from openviking.core.namespace import canonicalize_uri, is_content_namespace_root_uri +from openviking.core.namespace import canonicalize_uri from openviking.core.namespace import ( is_accessible as namespace_is_accessible, ) @@ -288,32 +288,6 @@ def bind_request_context(self, ctx: RequestContext): finally: self._bound_ctx.reset(token) - async def ensure_parent_exists( - self, - parent: str, - ctx: RequestContext, - *, - create_parent: bool = True, - ) -> None: - try: - stat_result = await self.stat(parent, ctx=ctx) - except Exception as exc: - if not isinstance(exc, (FileNotFoundError, NotFoundError)) and not is_not_found_error( - exc - ): - raise - if create_parent or is_content_namespace_root_uri(parent, ctx): - await self.mkdir(parent, exist_ok=True, ctx=ctx) - stat_result = await self.stat(parent, ctx=ctx) - else: - raise FileNotFoundError( - f"Parent URI does not exist: {parent}. " - "Use --parent-auto-create/-p to automatically create it." - ) from exc - - if not stat_result.get("isDir"): - raise InvalidArgumentError(f"Parent URI is not a directory: {parent}") - @staticmethod def _normalize_uri(uri: str) -> str: """Normalize short-format URIs to the canonical viking:// form.""" From bdc92f0007a9b5cbddfd2dacaf1f7ec4d1aa5e85 Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Fri, 12 Jun 2026 14:27:55 +0800 Subject: [PATCH 6/7] docs: update user content target docs --- README.md | 27 ++++++----- README_CN.md | 27 ++++++----- docs/en/api/02-resources.md | 47 +++++++++++++++---- docs/en/api/04-skills.md | 29 +++++++----- docs/en/api/06-retrieval.md | 24 +++++++++- docs/en/concepts/04-viking-uri.md | 42 +++++++++++++---- docs/en/concepts/11-multi-tenant.md | 22 +++++++-- docs/zh/api/02-resources.md | 46 ++++++++++++++---- docs/zh/api/04-skills.md | 28 ++++++----- docs/zh/api/06-retrieval.md | 24 +++++++++- docs/zh/concepts/04-viking-uri.md | 46 ++++++++++++++---- docs/zh/concepts/11-multi-tenant.md | 21 +++++++-- .../skills/ov-resources/docs/add-resource.md | 12 ++++- 13 files changed, 299 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index 281e9b1eec..85ce7873fd 100644 --- a/README.md +++ b/README.md @@ -705,18 +705,21 @@ viking:// │ │ └── src/ │ └── ... ├── user/ # User: personal preferences, habits, etc. -│ └── memories/ -│ ├── preferences/ -│ │ ├── writing_style -│ │ └── coding_habits -│ └── ... -└── agent/ # Agent: skills, instructions, task memories, etc. - ├── skills/ - │ ├── search_code - │ ├── analyze_data - │ └── ... - ├── memories/ - └── instructions/ +│ └── {user_id}/ +│ ├── memories/ +│ │ ├── preferences/ +│ │ │ ├── writing_style +│ │ │ └── coding_habits +│ │ └── ... +│ ├── resources/ +│ │ └── private_project/ +│ ├── skills/ +│ │ ├── search_code +│ │ └── analyze_data +│ └── peers/ +│ └── web-visitor-alice/ +│ ├── memories/ +│ └── resources/ ``` ### 2. Tiered Context Loading → Reduces Token Consumption diff --git a/README_CN.md b/README_CN.md index c69927af06..4a95d4bf1d 100644 --- a/README_CN.md +++ b/README_CN.md @@ -749,18 +749,21 @@ viking:// │ │ └── src/ │ └── ... ├── user/ # 用户：个人偏好、习惯等 -│ └── memories/ -│ ├── preferences/ -│ │ ├── writing_style -│ │ └── coding_habits -│ └── ... -└── agent/ # 智能体：技能、指令、任务记忆等 - ├── skills/ - │ ├── search_code - │ ├── analyze_data - │ └── ... - ├── memories/ - └── instructions/ +│ └── {user_id}/ +│ ├── memories/ +│ │ ├── preferences/ +│ │ │ ├── writing_style +│ │ │ └── coding_habits +│ │ └── ... +│ ├── resources/ +│ │ └── private_project/ +│ ├── skills/ +│ │ ├── search_code +│ │ └── analyze_data +│ └── peers/ +│ └── web-visitor-alice/ +│ ├── memories/ +│ └── resources/ ``` ### 2. 分层上下文加载 → 降低 Token 消耗 diff --git a/docs/en/api/02-resources.md b/docs/en/api/02-resources.md index 0c7457f402..8717b89633 100644 --- a/docs/en/api/02-resources.md +++ b/docs/en/api/02-resources.md @@ -161,6 +161,8 @@ This endpoint is the core entry point for resource management, supporting adding **Additional Notes**: - `to` and `parent` cannot be specified together. Use `create_parent=true` with `parent` when the parent directory should be created automatically. +- Resource targets may use public `viking://resources/...`, current-user shorthand `viking://user/resources/...`, explicit user `viking://user/{user_id}/resources/...`, or peer `viking://user/{user_id}/peers/{peer_id}/resources/...` paths. Current-user shorthand is canonicalized with the authenticated request identity. +- `user_id` and `peer_id` path segments must be safe single-segment identifiers, for example `alice` or `web-visitor-alice`. Values with path separators, `.`, `..`, `:`, or `+` are rejected. - `path` and `temp_file_id` cannot be specified together - Raw HTTP calls for local files require first uploading via [temp_upload](#temp_upload) to obtain `temp_file_id` - When `to` is specified and the target already exists, triggers incremental update @@ -206,6 +208,16 @@ curl -X POST http://localhost:1933/api/v1/resources \ \"to\": \"viking://resources/guide.md\", \"reason\": \"User guide\" }" + +# Add to the current user's private resource root +curl -X POST http://localhost:1933/api/v1/resources \ + -H "Content-Type: application/json" \ + -H "X-API-Key: your-key" \ + -d "{ + \"temp_file_id\": \"$TEMP_FILE_ID\", + \"parent\": \"viking://user/resources/docs\", + \"create_parent\": true + }" ``` **Python SDK** @@ -235,6 +247,13 @@ result = client.add_resource( reason="External API documentation" ) +# Add to the current user's private resource root +result = client.add_resource( + "./documents/guide.md", + parent="viking://user/resources/docs", + create_parent=True, +) + # Wait for processing to complete client.wait_processed() @@ -270,6 +289,13 @@ ov add-resource https://github.com/example/repo.git --to viking://resources/guid # Add with parent directory (parent must exist) ov add-resource ./documents/guide.md --parent viking://resources/docs +# Add under the current user's private resource root +ov add-resource ./documents/guide.md --parent viking://user/resources/docs + +# Add under a specific peer's private resource root +ov add-resource ./documents/guide.md \ + --parent viking://user/alice/peers/web-visitor-alice/resources/docs + # Add with parent directory (auto-create parent if it doesn't exist) ov add-resource ./documents/guide.md -p viking://resources/docs/2026/05/07 # Or using full flag @@ -488,6 +514,11 @@ Skills are special resources used to define operations or tools that agents can | timeout | float | No | None | Timeout in seconds, only effective when `wait=True` | | telemetry | TelemetryRequest | No | False | Whether to return telemetry data | +Skills are always installed under the current user's skills root. The public short form +`viking://user/skills` is accepted for filesystem/search operations and resolves to +`viking://user/{user_id}/skills`; `add_skill` does not accept `to`, `parent`, +`root_uri`, or peer-scoped skill targets. + #### 3. Usage Examples **HTTP API** @@ -560,8 +591,8 @@ ov add-skill ./skills/my-skill.json --wait "status": "ok", "result": { "status": "success", - "root_uri": "viking://user/skills/my-skill", - "uri": "viking://user/skills/my-skill", + "root_uri": "viking://user/alice/skills/my-skill", + "uri": "viking://user/alice/skills/my-skill", "name": "my-skill", "auxiliary_files": 2, "queue_status": { @@ -582,8 +613,8 @@ ov add-skill ./skills/my-skill.json --wait Note: Skill is being processed in the background. Use 'ov wait' to wait for completion, or 'ov observer queue' to check status. status success -root_uri viking://user/skills/my-skill -uri viking://user/skills/my-skill +root_uri viking://user/alice/skills/my-skill +uri viking://user/alice/skills/my-skill name my-skill auxiliary_files 2 ``` @@ -593,8 +624,8 @@ auxiliary_files 2 ```json { "status": "success", - "root_uri": "viking://user/skills/my-skill", - "uri": "viking://user/skills/my-skill", + "root_uri": "viking://user/alice/skills/my-skill", + "uri": "viking://user/alice/skills/my-skill", "name": "my-skill", "auxiliary_files": 2 } @@ -605,8 +636,8 @@ auxiliary_files 2 | Field | Type | Description | |-------|------|-------------| | `status` | string | Processing status: "success" or "error" | -| `root_uri` | string | Final URI of the skill in OpenViking (same as `uri`) | -| `uri` | string | Final URI of the skill in OpenViking (same as `root_uri`) | +| `root_uri` | string | Canonical final URI of the skill in OpenViking (same as `uri`) | +| `uri` | string | Canonical final URI of the skill in OpenViking (same as `root_uri`) | | `name` | string | Skill name | | `auxiliary_files` | number | Number of auxiliary files attached to the skill | | `queue_status` | object | (Optional, only when `wait=true`) Queue processing status with `pending`, `processing`, `completed` counts | diff --git a/docs/en/api/04-skills.md b/docs/en/api/04-skills.md index d5c1736560..94acc12dcb 100644 --- a/docs/en/api/04-skills.md +++ b/docs/en/api/04-skills.md @@ -14,10 +14,12 @@ OpenViking supports multiple skill definition formats: ### Skill Storage Structure -Skills are stored at `viking://user/skills/`: +Skills are stored under the current user's skills root. The short URI +`viking://user/skills/` resolves to `viking://user/{user_id}/skills/` for the +authenticated request: ``` -viking://user/skills/ +viking://user/{user_id}/skills/ +-- search-web/ | +-- .abstract.md # L0: Brief description | +-- .overview.md # L1: Parameters and usage overview @@ -152,7 +154,7 @@ Skills are a special type of resource that define actions or tools agents can pe 1. Receive skill data or uploaded temporary file 2. Detect data format (structured data, SKILL.md content, MCP format) 3. Parse skill definition -4. Store to `viking://user/skills/` path +4. Store to the current user's `viking://user/{user_id}/skills/` path 5. If `wait=true`, wait for vectorization to complete **Code Entry Points**: @@ -184,6 +186,11 @@ Skills are a special type of resource that define actions or tools agents can pe - `temp_upload` defaults to local temporary storage; pass `upload_mode=shared` only when you explicitly need distributed shared temporary uploads. In Python HTTP client / CLI flows, this can also be driven by `ovcli.conf` via `upload.mode = "shared"` - `POST /api/v1/skills` does not accept direct host filesystem paths in `data`. +- **Targeting**: + - Skills are always user-scoped. `add_skill` does not accept `to`, `parent`, or `root_uri`. + - Peer-scoped skill roots are not supported; peer-aware retrieval only adds peer memories/resources, not peer skills. + - Use `viking://user/skills/...` as current-user shorthand when listing, reading, deleting, or searching skills. + - **Supported data formats**: 1. **Dict (Skill format)**: Includes `name`, `description`, `content`, etc. 2. **Dict (MCP Tool format)**: Includes `name`, `description`, `inputSchema`, auto-detected and converted @@ -337,8 +344,8 @@ ov add-skill ./skills/my-skill/ -o json "status": "ok", "result": { "status": "success", - "root_uri": "viking://user/skills/my-skill/", - "uri": "viking://user/skills/my-skill/", + "root_uri": "viking://user/alice/skills/my-skill", + "uri": "viking://user/alice/skills/my-skill", "name": "my-skill", "auxiliary_files": 2, "queue_status": { @@ -359,8 +366,8 @@ ov add-skill ./skills/my-skill/ -o json Note: Skill is being processed in the background. Use 'ov wait' to wait for completion, or 'ov observer queue' to check status. status success -root_uri viking://user/skills/my-skill -uri viking://user/skills/my-skill +root_uri viking://user/alice/skills/my-skill +uri viking://user/alice/skills/my-skill name my-skill auxiliary_files 2 ``` @@ -369,8 +376,8 @@ auxiliary_files 2 ```json { "status": "success", - "root_uri": "viking://user/skills/my-skill", - "uri": "viking://user/skills/my-skill", + "root_uri": "viking://user/alice/skills/my-skill", + "uri": "viking://user/alice/skills/my-skill", "name": "my-skill", "auxiliary_files": 2 } @@ -381,8 +388,8 @@ auxiliary_files 2 | Field | Type | Description | |-------|------|-------------| | `status` | string | Processing status: `success` or `error` | -| `root_uri` | string | Final URI of the skill in OpenViking (same as `uri`) | -| `uri` | string | Final URI of the skill in OpenViking (same as `root_uri`) | +| `root_uri` | string | Canonical final URI of the skill in OpenViking (same as `uri`) | +| `uri` | string | Canonical final URI of the skill in OpenViking (same as `root_uri`) | | `name` | string | Skill name | | `auxiliary_files` | number | Number of auxiliary files included with the skill | | `queue_status` | object | (Optional, only when `wait=true`) Queue processing status with `pending`, `processing`, `completed` counts | diff --git a/docs/en/api/06-retrieval.md b/docs/en/api/06-retrieval.md index 537d07a91a..3f9bf35d52 100644 --- a/docs/en/api/06-retrieval.md +++ b/docs/en/api/06-retrieval.md @@ -56,7 +56,7 @@ The `find()` method performs pure vector similarity search for simple query scen |-----------|------|----------|---------|-------------| | query | str | Yes | - | Search query string | | target_uri | str \| List[str] | No | "" | Limit search to specific URI prefix | -| peer_id | str | No | None | Stable interaction peer ID. When searching the default user memory scope, results include both current user memories and this peer's memories. CLI maps `--peer-id` to this field | +| peer_id | str | No | None | Stable interaction peer ID. When searching default user-scoped targets, results include this peer's memories and resources in addition to current-user content. CLI maps `--peer-id` to this field | | node_limit | int | No | None | Maximum number of results | | score_threshold | float | No | None | Minimum relevance score threshold | | filter | Dict | No | None | Metadata filter | @@ -67,6 +67,12 @@ The `find()` method performs pure vector similarity search for simple query scen | include_provenance | bool | No | False | Include provenance/query-plan details in serialized result | | telemetry | bool \| object | No | False | Attach telemetry data to response | +**Target resolution notes**: +- With empty `target_uri`, non-ROOT retrieval searches current-user memories, shared `viking://resources`, current-user resources, and current-user skills. +- When `peer_id` is provided, OpenViking also searches `viking://user/{user_id}/peers/{peer_id}/memories` and `viking://user/{user_id}/peers/{peer_id}/resources`. Peer skills are not searched. +- `peer_id` must be a safe single path segment, for example `web-visitor-alice`; values such as `web:visitor:alice`, `web+visitor+alice`, `.`, `..`, or values with path separators are rejected. +- Current-user shorthand target URIs such as `viking://user/memories`, `viking://user/resources`, and `viking://user/skills` are canonicalized from the authenticated request identity. + **FindResult Structure** ```python @@ -169,6 +175,18 @@ results = client.find( target_uri="viking://user/memories" ) +# Search only in current-user resources +results = client.find( + "private docs", + target_uri="viking://user/resources" +) + +# Include a specific peer's memories and resources in default retrieval +results = client.find( + "invoice follow-up", + peer_id="web-visitor-alice" +) + # Search only in skills results = client.find( "web search", @@ -274,7 +292,7 @@ The `search()` method adds session context understanding and intent analysis cap | target_uri | str \| List[str] | No | "" | Limit search to specific URI prefix | | session | Session | No | None | Session for context-aware search (SDK) | | session_id | str | No | None | Session ID for context-aware search (HTTP) | -| peer_id | str | No | None | Stable interaction peer ID. When searching the default user memory scope, results include both current user memories and this peer's memories. CLI maps `--peer-id` to this field | +| peer_id | str | No | None | Stable interaction peer ID. When searching default user-scoped targets, results include this peer's memories and resources in addition to current-user content. CLI maps `--peer-id` to this field | | node_limit | int | No | None | Maximum number of results | | score_threshold | float | No | None | Minimum relevance score threshold | | filter | Dict | No | None | Metadata filter | @@ -285,6 +303,8 @@ The `search()` method adds session context understanding and intent analysis cap | include_provenance | bool | No | False | Include provenance/query-plan details in serialized result | | telemetry | bool \| object | No | False | Attach telemetry data to response | +`search()` uses the same target resolution rules as `find()`: default retrieval includes current-user memories/resources/skills plus shared resources, and `peer_id` adds only that peer's memories/resources. + #### 3. Usage Examples **HTTP API** diff --git a/docs/en/concepts/04-viking-uri.md b/docs/en/concepts/04-viking-uri.md index a588e646c1..762ba41ee7 100644 --- a/docs/en/concepts/04-viking-uri.md +++ b/docs/en/concepts/04-viking-uri.md @@ -39,7 +39,12 @@ viking:// │ └── {user_id}/ │ ├── profile.md # User profile │ ├── memories/ # User memory storage +│ ├── resources/ # User-owned private resources │ ├── skills/ # User skills +│ ├── peers/ +│ │ └── {peer_id}/ +│ │ ├── memories/ # Memory about a specific interaction peer +│ │ └── resources/ # Resources scoped to that peer │ └── sessions/ # User session storage │ └── {session_id}/ │ ├── .abstract.md @@ -72,9 +77,11 @@ viking://user/memories/preferences/ # User preferences viking://user/memories/preferences/coding # Specific preference viking://user/memories/entities/ # Entity memories viking://user/memories/events/ # Event memories +viking://user/resources/ # Current user's resources +viking://user/resources/docs/ # Current user's resource directory ``` -### User Skills and Memories +### User Skills and Peer Content ``` viking://user/skills/ # Current user's skills @@ -82,11 +89,15 @@ viking://user/skills/search-web # Specific skill viking://user/memories/ # Current user's memories viking://user/memories/cases/ # Learned cases viking://user/memories/patterns/ # Learned patterns +viking://user/{user_id}/peers/{peer_id}/memories/ +viking://user/{user_id}/peers/{peer_id}/resources/ ``` The short `viking://user/...` form is relative to the current request identity. OpenViking expands it internally to explicit namespace paths such as `viking://user/{user_id}/...` before storage and retrieval. +Identity path segments such as `{user_id}` and `{peer_id}` must be safe single +segments, for example `alice` or `web-visitor-alice`. ### Session Data @@ -189,10 +200,16 @@ viking:// │ ├── user/{user_id}/ │ ├── profile.md # User basic info -│ └── memories/ -│ ├── preferences/ # By topic -│ ├── entities/ # Each independent -│ └── events/ # Each independent +│ ├── memories/ +│ │ ├── preferences/ # By topic +│ │ ├── entities/ # Each independent +│ │ └── events/ # Each independent +│ ├── resources/ +│ │ └── {project}/ +│ ├── skills/ +│ └── peers/{peer_id}/ +│ ├── memories/ +│ └── resources/ │ └── user/{user_id}/sessions/{session_id}/ ├── messages.jsonl @@ -237,6 +254,12 @@ results = client.find( target_uri="viking://resources/" ) +# Search only in current-user resources +results = client.find( + "private project notes", + target_uri="viking://user/resources/" +) + # Search only in user memories results = client.find( "coding preferences", @@ -292,11 +315,14 @@ Each directory may contain special files: ### Scope-Specific Operations ```python -# Add resources only to resources scope +# Add resources to the shared account resource scope await client.add_resource(url, to="viking://resources/project/") -# Skills go to user scope -await client.add_skill(skill) # Automatically to viking://user/skills/ +# Add private resources to the current user's resource root +await client.add_resource(path, parent="viking://user/resources/project/") + +# Skills always go to the current user's skills root +await client.add_skill(skill) # Canonical root: viking://user/{user_id}/skills/ ``` ## Related Documents diff --git a/docs/en/concepts/11-multi-tenant.md b/docs/en/concepts/11-multi-tenant.md index e66fd31b1b..db5afc2923 100644 --- a/docs/en/concepts/11-multi-tenant.md +++ b/docs/en/concepts/11-multi-tenant.md @@ -73,9 +73,12 @@ If `auth_mode = "api_key"` and `root_api_key` is not configured, the server runs | Data type | Shared across accounts | Shared inside one account | Default isolation boundary | |-----------|------------------------|---------------------------|----------------------------| -| `resources` | No | Yes | account | -| `user` | No | No | user | -| `session` | No | No | user / session | +| Shared resources (`viking://resources`) | No | Yes | account | +| User resources (`viking://user/{user_id}/resources`) | No | No | user | +| Peer resources (`viking://user/{user_id}/peers/{peer_id}/resources`) | No | No | user / peer | +| Memories | No | No | user / peer | +| Skills | No | No | user | +| Sessions | No | No | user / session | ### Storage Layer @@ -84,6 +87,8 @@ For users, URIs still look like normal `viking://...` paths: ```text viking://resources/project-a/ viking://user/alice/memories/ +viking://user/alice/resources/ +viking://user/alice/peers/web-visitor-alice/resources/ ``` But the underlying storage automatically gains an account prefix: @@ -91,6 +96,8 @@ But the underlying storage automatically gains an account prefix: ```text /local/{account_id}/resources/project-a/ /local/{account_id}/user/alice/memories/ +/local/{account_id}/user/alice/resources/ +/local/{account_id}/user/alice/peers/web-visitor-alice/resources/ ``` So multi-tenant isolation does not rely on a special public URI format. It relies on request context, `account_id` and `user_id`, applied consistently through the stack. @@ -101,7 +108,8 @@ Semantic retrieval is tenant-aware as well: - Non-ROOT requests are automatically filtered by `account_id` - `resources` can include account-shared resources -- `memory` and `skill` are further filtered by the current `user space` +- `memory`, user resources, and `skill` are further filtered by the current user space +- Passing `peer_id` adds only that peer's memories/resources; skills remain user-scoped This keeps "what you can search" aligned with "what you can read." @@ -268,10 +276,14 @@ caller identity it should run as. ### 2. `peer_id` does not define the tenant -`peer_id` identifies the message peer in shared conversations. It does not create a tenant or filesystem namespace. +`peer_id` identifies an interaction peer under the current user. It does not create a tenant, +but it can select a peer content subspace such as +`viking://user/{user_id}/peers/{peer_id}/memories` or +`viking://user/{user_id}/peers/{peer_id}/resources`. - The tenant boundary is `account_id` - The user boundary is `user_id` +- Peer content remains inside that user boundary ### 3. No `root_api_key` does not mean "formal single-tenant production mode" diff --git a/docs/zh/api/02-resources.md b/docs/zh/api/02-resources.md index 87ded1b511..eb2c301481 100644 --- a/docs/zh/api/02-resources.md +++ b/docs/zh/api/02-resources.md @@ -156,6 +156,8 @@ URL/文件 Parser TreeBuilder AGFS Summarizer/Vector **补充说明**： - `to` 和 `parent` 不能同时使用；如果使用 `parent` 且希望父目录不存在时自动创建，请传 `create_parent=true`。指定 `to` 且目标已存在时，触发增量更新。 +- 资源目标可以使用公共 `viking://resources/...`、当前用户短写 `viking://user/resources/...`、显式用户 `viking://user/{user_id}/resources/...`，或 peer 级 `viking://user/{user_id}/peers/{peer_id}/resources/...`。当前用户短写会按请求身份 canonicalize。 +- `user_id` 和 `peer_id` 路径片段必须是安全的单段标识，例如 `alice` 或 `web-visitor-alice`。包含路径分隔符、`.`、`..`、`:` 或 `+` 的值会被拒绝。 - `path` 和 `temp_file_id` 不能同时指定，上传本地文件需要先通过 [temp_upload](#temp_upload) 上传获取 `temp_file_id`，在 SDK 和 CLI 中已经封装好。 - 只有 Git 仓库来源在 `wait=false` 时使用完整后台导入；OpenViking 会先完成仓库 preflight 和目标规划，再返回 `task_id`。 - 其他来源在 `wait=false` 时会在响应前完成来源解析、目标解析和 AGFS 写入，仅 semantic 与 embedding 队列继续异步处理。 @@ -199,6 +201,16 @@ curl -X POST http://localhost:1933/api/v1/resources \ \"to\": \"viking://resources/guide.md\", \"reason\": \"User guide\" }" + +# 添加到当前用户私有资源根 +curl -X POST http://localhost:1933/api/v1/resources \ + -H "Content-Type: application/json" \ + -H "X-API-Key: your-key" \ + -d "{ + \"temp_file_id\": \"$TEMP_FILE_ID\", + \"parent\": \"viking://user/resources/docs\", + \"create_parent\": true + }" ``` **Python SDK** @@ -228,6 +240,13 @@ result = client.add_resource( reason="External API docs" ) +## 添加到当前用户私有资源根 +result = client.add_resource( + "./documents/guide.md", + parent="viking://user/resources/docs", + create_parent=True, +) + ## 等待处理完成 client.wait_processed() @@ -263,6 +282,13 @@ ov add-resource https://github.com/example/repo.git --to viking://resources/my_r # 添加到指定父目录（父目录必须存在） ov add-resource ./documents/guide.md --parent viking://resources/docs +# 添加到当前用户私有资源根 +ov add-resource ./documents/guide.md --parent viking://user/resources/docs + +# 添加到指定 peer 的私有资源根 +ov add-resource ./documents/guide.md \ + --parent viking://user/alice/peers/web-visitor-alice/resources/docs + # 添加到指定父目录（父目录不存在时自动创建） ov add-resource ./documents/guide.md -p viking://resources/docs/2026/05/07 # 或使用完整参数名 @@ -481,6 +507,10 @@ cancel_watch(to_uri="viking://resources/guide.md") # 按 URI 幂等删除 | timeout | float | 否 | None | 超时时间（秒），仅 `wait=true` 时生效 | | telemetry | TelemetryRequest | 否 | False | 是否返回遥测数据 | +技能始终安装到当前用户的 skills 根。公共短写 `viking://user/skills` 可用于文件系统和检索操作， +会解析为 `viking://user/{user_id}/skills`；`add_skill` 不接受 `to`、`parent`、`root_uri` +或 peer-scoped skill 目标。 + #### 3. 使用示例 **HTTP API** @@ -553,8 +583,8 @@ ov add-skill ./skills/my-skill.json --wait "status": "ok", "result": { "status": "success", - "root_uri": "viking://user/skills/my-skill", - "uri": "viking://user/skills/my-skill", + "root_uri": "viking://user/alice/skills/my-skill", + "uri": "viking://user/alice/skills/my-skill", "name": "my-skill", "auxiliary_files": 2, "queue_status": { @@ -575,8 +605,8 @@ ov add-skill ./skills/my-skill.json --wait Note: Skill is being processed in the background. Use 'ov wait' to wait for completion, or 'ov observer queue' to check status. status success -root_uri viking://user/skills/my-skill -uri viking://user/skills/my-skill +root_uri viking://user/alice/skills/my-skill +uri viking://user/alice/skills/my-skill name my-skill auxiliary_files 2 ``` @@ -586,8 +616,8 @@ auxiliary_files 2 ```json { "status": "success", - "root_uri": "viking://user/skills/my-skill", - "uri": "viking://user/skills/my-skill", + "root_uri": "viking://user/alice/skills/my-skill", + "uri": "viking://user/alice/skills/my-skill", "name": "my-skill", "auxiliary_files": 2 } @@ -598,8 +628,8 @@ auxiliary_files 2 | 字段 | 类型 | 说明 | |------|------|------| | `status` | string | 处理状态："success" 成功，"error" 失败 | -| `root_uri` | string | 技能在 OpenViking 中的最终 URI（同 `uri`） | -| `uri` | string | 技能在 OpenViking 中的最终 URI（同 `root_uri`） | +| `root_uri` | string | 技能在 OpenViking 中的 canonical 最终 URI（同 `uri`） | +| `uri` | string | 技能在 OpenViking 中的 canonical 最终 URI（同 `root_uri`） | | `name` | string | 技能名称 | | `auxiliary_files` | number | 技能附带的辅助文件数量 | | `queue_status` | object | （可选，仅当 `wait=true` 时）队列处理状态，包含 `pending`、`processing`、`completed` 计数 | diff --git a/docs/zh/api/04-skills.md b/docs/zh/api/04-skills.md index 251b7f7b01..47485e282f 100644 --- a/docs/zh/api/04-skills.md +++ b/docs/zh/api/04-skills.md @@ -14,10 +14,11 @@ OpenViking 支持多种技能定义格式： ### 技能存储结构 -技能存储在 `viking://user/skills/` 路径下： +技能存储在当前用户的 skills 根。短 URI `viking://user/skills/` 会按认证请求身份解析为 +`viking://user/{user_id}/skills/`： ``` -viking://user/skills/ +viking://user/{user_id}/skills/ +-- search-web/ | +-- .abstract.md # L0：简要描述 | +-- .overview.md # L1：参数和使用概览 @@ -152,7 +153,7 @@ This tool wraps the MCP tool `search-web`. Call this when the user needs functio 1. 接收技能数据或上传的临时文件 2. 检测数据格式（结构化数据、SKILL.md 内容、MCP 格式） 3. 解析技能定义 -4. 存储到 `viking://user/skills/` 路径下 +4. 存储到当前用户的 `viking://user/{user_id}/skills/` 路径下 5. 如指定 `wait=True`，等待向量化完成 **代码入口**： @@ -185,6 +186,11 @@ This tool wraps the MCP tool `search-web`. Call this when the user needs functio 4. `temp_upload` 默认使用本地临时存储；只有在明确需要分布式共享临时上传时，才传 `upload_mode=shared`。在 Python HTTP client / CLI 流程里，也可以通过 `ovcli.conf` 的 `upload.mode = "shared"` 驱动这一行为 - `POST /api/v1/skills` 不接受在 `data` 中直接传宿主机本地路径。 +- **目标规则**： + - Skills 始终是 user-scoped；`add_skill` 不接受 `to`、`parent` 或 `root_uri`。 + - 不支持 peer-scoped skill 根；带 `peer_id` 的检索只额外加入 peer memories/resources，不加入 peer skills。 + - 列出、读取、删除或搜索技能时，可以使用 `viking://user/skills/...` 作为当前用户短写。 + - **支持的数据格式**： 1. **字典（技能格式）**：包含 `name`、`description`、`content` 等字段 2. **字典（MCP Tool 格式）**：包含 `name`、`description`、`inputSchema` 字段，会自动检测并转换 @@ -338,8 +344,8 @@ ov add-skill ./skills/my-skill/ -o json "status": "ok", "result": { "status": "success", - "root_uri": "viking://user/skills/my-skill/", - "uri": "viking://user/skills/my-skill/", + "root_uri": "viking://user/alice/skills/my-skill", + "uri": "viking://user/alice/skills/my-skill", "name": "my-skill", "auxiliary_files": 2, "queue_status": { @@ -360,8 +366,8 @@ ov add-skill ./skills/my-skill/ -o json Note: Skill is being processed in the background. Use 'ov wait' to wait for completion, or 'ov observer queue' to check status. status success -root_uri viking://user/skills/my-skill -uri viking://user/skills/my-skill +root_uri viking://user/alice/skills/my-skill +uri viking://user/alice/skills/my-skill name my-skill auxiliary_files 2 ``` @@ -370,8 +376,8 @@ auxiliary_files 2 ```json { "status": "success", - "root_uri": "viking://user/skills/my-skill", - "uri": "viking://user/skills/my-skill", + "root_uri": "viking://user/alice/skills/my-skill", + "uri": "viking://user/alice/skills/my-skill", "name": "my-skill", "auxiliary_files": 2 } @@ -382,8 +388,8 @@ auxiliary_files 2 | 字段 | 类型 | 说明 | |------|------|------| | `status` | string | 处理状态：`success` 成功，`error` 失败 | -| `root_uri` | string | 技能在 OpenViking 中的最终 URI（同 `uri`）| -| `uri` | string | 技能在 OpenViking 中的最终 URI（同 `root_uri`）| +| `root_uri` | string | 技能在 OpenViking 中的 canonical 最终 URI（同 `uri`）| +| `uri` | string | 技能在 OpenViking 中的 canonical 最终 URI（同 `root_uri`）| | `name` | string | 技能名称 | | `auxiliary_files` | number | 技能附带的辅助文件数量 | | `queue_status` | object | （可选，仅当 `wait=True` 时）队列处理状态，包含 `pending`、`processing`、`completed` 计数 | diff --git a/docs/zh/api/06-retrieval.md b/docs/zh/api/06-retrieval.md index 870a21e6e5..24611e8bc6 100644 --- a/docs/zh/api/06-retrieval.md +++ b/docs/zh/api/06-retrieval.md @@ -56,7 +56,7 @@ OpenViking 提供多种检索方法，包括简单的向量相似度搜索、带 |------|------|------|--------|------| | query | str | 是 | - | 搜索查询字符串 | | target_uri | str \| List[str] | 否 | "" | 限制搜索范围到指定的 URI 前缀 | -| peer_id | str | 否 | None | 稳定交互对象 ID。检索默认 user memory 范围时，会同时检索当前 user memory 和该 peer 的 memory。CLI `--peer-id` 会映射到这个字段 | +| peer_id | str | 否 | None | 稳定交互对象 ID。检索默认 user-scoped 目标时，会在当前用户内容之外额外检索该 peer 的 memories 和 resources。CLI `--peer-id` 会映射到这个字段 | | limit | int | 否 | 10 | 最大返回结果数 | | node_limit | int | 否 | None | 可选 HTTP 别名；如果提供，会覆盖 limit | | score_threshold | float | 否 | None | 最低相关性分数阈值 | @@ -68,6 +68,12 @@ OpenViking 提供多种检索方法，包括简单的向量相似度搜索、带 | include_provenance | bool | 否 | False | 在序列化结果中附带 provenance / query-plan 细节 | | telemetry | bool \| object | 否 | False | 在响应中附带遥测数据 | +**目标解析说明**： +- `target_uri` 为空时，非 ROOT 检索默认搜索当前用户 memories、公共 `viking://resources`、当前用户 resources 和当前用户 skills。 +- 传入 `peer_id` 时，OpenViking 会额外搜索 `viking://user/{user_id}/peers/{peer_id}/memories` 和 `viking://user/{user_id}/peers/{peer_id}/resources`，不会搜索 peer skills。 +- `peer_id` 必须是安全的单段路径标识，例如 `web-visitor-alice`；`web:visitor:alice`、`web+visitor+alice`、`.`、`..` 或包含路径分隔符的值会被拒绝。 +- `viking://user/memories`、`viking://user/resources`、`viking://user/skills` 等当前用户短写 target URI 会按认证请求身份 canonicalize。 + **FindResult 结构** ```python @@ -170,6 +176,18 @@ results = client.find( target_uri="viking://user/memories" ) +# 仅在当前用户资源中搜索 +results = client.find( + "private docs", + target_uri="viking://user/resources" +) + +# 在默认检索范围中额外加入指定 peer 的 memories/resources +results = client.find( + "invoice follow-up", + peer_id="web-visitor-alice" +) + # 仅在技能中搜索 results = client.find( "web search", @@ -275,7 +293,7 @@ openviking find "how to authenticate users" -L 1,2 | target_uri | str \| List[str] | 否 | "" | 限制搜索范围到指定的 URI 前缀 | | session | Session | 否 | None | 用于上下文感知搜索的会话（SDK）| | session_id | str | 否 | None | 用于上下文感知搜索的会话 ID（HTTP）| -| peer_id | str | 否 | None | 稳定交互对象 ID。检索默认 user memory 范围时，会同时检索当前 user memory 和该 peer 的 memory。CLI `--peer-id` 会映射到这个字段 | +| peer_id | str | 否 | None | 稳定交互对象 ID。检索默认 user-scoped 目标时，会在当前用户内容之外额外检索该 peer 的 memories 和 resources。CLI `--peer-id` 会映射到这个字段 | | limit | int | 否 | 10 | 最大返回结果数 | | node_limit | int | 否 | None | 可选 HTTP 别名；如果提供，会覆盖 limit | | score_threshold | float | 否 | None | 最低相关性分数阈值 | @@ -287,6 +305,8 @@ openviking find "how to authenticate users" -L 1,2 | include_provenance | bool | 否 | False | 在序列化结果中附带 provenance / query-plan 细节 | | telemetry | bool \| object | 否 | False | 在响应中附带遥测数据 | +`search()` 使用和 `find()` 相同的目标解析规则：默认检索包含当前用户 memories/resources/skills 和公共 resources，`peer_id` 只额外加入该 peer 的 memories/resources。 + #### 3. 使用示例 **HTTP API** diff --git a/docs/zh/concepts/04-viking-uri.md b/docs/zh/concepts/04-viking-uri.md index bbfcada4bf..431ae0a735 100644 --- a/docs/zh/concepts/04-viking-uri.md +++ b/docs/zh/concepts/04-viking-uri.md @@ -37,7 +37,12 @@ viking:// │ └── {user_id}/ │ ├── profile.md # 用户画像 │ ├── memories/ # 用户记忆 +│ ├── resources/ # 用户私有资源 │ ├── skills/ # 用户技能 +│ ├── peers/ +│ │ └── {peer_id}/ +│ │ ├── memories/ # 关于某个交互对象的记忆 +│ │ └── resources/ # 归属于该 peer 的资源 │ └── sessions/ # 用户会话 │ └── {session_id}/ │ ├── .abstract.md @@ -70,9 +75,11 @@ viking://user/memories/preferences/ # 用户偏好 viking://user/memories/preferences/coding # 具体偏好 viking://user/memories/entities/ # 实体记忆 viking://user/memories/events/ # 事件记忆 +viking://user/resources/ # 当前用户资源 +viking://user/resources/docs/ # 当前用户资源目录 ``` -### 用户技能和记忆 +### 用户技能和 peer 内容 ``` viking://user/skills/ # 当前用户的技能 @@ -80,11 +87,15 @@ viking://user/skills/search-web # 某个技能 viking://user/memories/ # 当前用户的记忆 viking://user/memories/cases/ # 学习的案例 viking://user/memories/patterns/ # 学习的模式 +viking://user/{user_id}/peers/{peer_id}/memories/ +viking://user/{user_id}/peers/{peer_id}/resources/ ``` 上面的 `viking://user/...` 短路径会按当前请求身份解析。 OpenViking 会在存储和检索前将它展开为显式命名空间路径，例如 `viking://user/{user_id}/...`。 +`{user_id}` 和 `{peer_id}` 等身份路径片段必须是安全的单段标识，例如 +`alice` 或 `web-visitor-alice`。 ### 会话数据 @@ -184,12 +195,18 @@ viking:// │ ├── .overview.md # 概述 │ └── {files...} │ -├── user/ -│ ├── profile.md # 用户基本信息 -│ └── memories/ -│ ├── preferences/ # 按主题 -│ ├── entities/ # 每条独立 -│ └── events/ # 每条独立 +├── user/{user_id}/ +│ ├── profile.md # 用户基本信息 +│ ├── memories/ +│ │ ├── preferences/ # 按主题 +│ │ ├── entities/ # 每条独立 +│ │ └── events/ # 每条独立 +│ ├── resources/ +│ │ └── {project}/ +│ ├── skills/ +│ └── peers/{peer_id}/ +│ ├── memories/ +│ └── resources/ │ └── user/{user_id}/sessions/{session_id}/ ├── messages.jsonl @@ -234,6 +251,12 @@ results = client.find( target_uri="viking://resources/" ) +# 仅在当前用户资源中搜索 +results = client.find( + "私有项目笔记", + target_uri="viking://user/resources/" +) + # 仅在用户记忆中搜索 results = client.find( "编码偏好", @@ -289,11 +312,14 @@ overview = await client.overview("viking://resources/docs/") ### 作用域特定操作 ```python -# 资源只添加到 resources 作用域 +# 添加到 account 共享资源作用域 await client.add_resource(url, to="viking://resources/project/") -# 技能添加到 user 作用域 -await client.add_skill(skill) # 自动到 viking://user/skills/ +# 添加到当前用户私有资源根 +await client.add_resource(path, parent="viking://user/resources/project/") + +# 技能始终添加到当前用户技能根 +await client.add_skill(skill) # canonical root: viking://user/{user_id}/skills/ ``` ## 相关文档 diff --git a/docs/zh/concepts/11-multi-tenant.md b/docs/zh/concepts/11-multi-tenant.md index 4236fc460c..a83963a358 100644 --- a/docs/zh/concepts/11-multi-tenant.md +++ b/docs/zh/concepts/11-multi-tenant.md @@ -73,9 +73,12 @@ OpenViking Server 支持两种多租户相关认证模式： | 数据类型 | 是否跨 account 共享 | account 内是否共享 | 默认隔离边界 | |----------|---------------------|-------------------|--------------| -| `resources` | 否 | 是 | account | -| `user` | 否 | 否 | user | -| `session` | 否 | 否 | user / session | +| 共享资源 (`viking://resources`) | 否 | 是 | account | +| 用户资源 (`viking://user/{user_id}/resources`) | 否 | 否 | user | +| Peer 资源 (`viking://user/{user_id}/peers/{peer_id}/resources`) | 否 | 否 | user / peer | +| 记忆 | 否 | 否 | user / peer | +| 技能 | 否 | 否 | user | +| 会话 | 否 | 否 | user / session | ### 存储层 @@ -84,6 +87,8 @@ OpenViking Server 支持两种多租户相关认证模式： ```text viking://resources/project-a/ viking://user/alice/memories/ +viking://user/alice/resources/ +viking://user/alice/peers/web-visitor-alice/resources/ ``` 但底层存储会自动带上 account 前缀： @@ -91,6 +96,8 @@ viking://user/alice/memories/ ```text /local/{account_id}/resources/project-a/ /local/{account_id}/user/alice/memories/ +/local/{account_id}/user/alice/resources/ +/local/{account_id}/user/alice/peers/web-visitor-alice/resources/ ``` 因此多租户隔离不是靠“不同 URI 前缀”，而是靠请求上下文中的 `account_id` 和 `user_id` 共同生效。 @@ -101,7 +108,8 @@ viking://user/alice/memories/ - 非 ROOT 请求会自动按 `account_id` 过滤 - `resources` 会允许检索 account 内共享资源 -- `memory` 和 `skill` 会进一步按当前 `user space` 过滤 +- `memory`、用户资源和 `skill` 会进一步按当前 `user space` 过滤 +- 传入 `peer_id` 时只额外加入该 peer 的 memories/resources；skills 仍保持 user-scoped 这意味着“能搜到什么”与“能读到什么”保持一致，不会因为向量召回而越权。 @@ -265,10 +273,13 @@ Root key 主要用于： ### 2. `peer_id` 不决定 account -`peer_id` 表示共享会话里的消息说话人，不创建租户或文件系统 namespace。 +`peer_id` 表示当前用户下的交互对象。它不创建租户，但可以选择当前用户内的 peer 内容子空间， +例如 `viking://user/{user_id}/peers/{peer_id}/memories` 或 +`viking://user/{user_id}/peers/{peer_id}/resources`。 - account 边界由 `account_id` 决定 - user 边界由 `user_id` 决定 +- peer 内容仍位于该 user 边界内 ### 3. 不配置 `root_api_key` 不等于“单租户正式部署” diff --git a/examples/skills/ov-resources/docs/add-resource.md b/examples/skills/ov-resources/docs/add-resource.md index cc701a5587..d58e3d0490 100644 --- a/examples/skills/ov-resources/docs/add-resource.md +++ b/examples/skills/ov-resources/docs/add-resource.md @@ -1,6 +1,6 @@ # Resource Ingestion (`ov add-resource`) -The `ov add-resource` command imports external resources into OpenViking's context database under `viking://resources/`. +The `ov add-resource` command imports external resources into OpenViking's context database. By default it writes shared account resources under `viking://resources/`, and it can also target current-user or peer-scoped resource roots. ## Supported Sources @@ -33,7 +33,7 @@ ov add-resource ./docs-of-project.zip ## Target Location -By default resources go under `viking://resources/`. Use `--to` or `--parent` to override: +By default resources go under shared `viking://resources/`. Use `--to` or `--parent` to override: ```bash # Exact target (must not exist) @@ -42,11 +42,19 @@ ov add-resource ./docs --to "viking://resources/2026/2026-01-01/" # Place under existing parent directory ov add-resource ./docs --parent "viking://resources/docs/" +# Place under the current user's private resource root +ov add-resource ./docs --parent "viking://user/resources/docs/" + +# Place under a specific peer's private resource root +ov add-resource ./docs --parent "viking://user/alice/peers/web-visitor-alice/resources/docs/" + # Auto-create parent if missing ov add-resource ./docs --parent-auto-create "viking://resources/docs/2026/05/07" ov add-resource ./docs -p "viking://resources/docs/{calendar:today}" ``` +`viking://user/resources/...` is current-user shorthand and resolves to `viking://user/{user_id}/resources/...`. `peer_id` path segments must be safe single-segment identifiers such as `web-visitor-alice`; values with `:`, `+`, `.`, `..`, or path separators are rejected. + ## Async Processing Control Semantic processing runs asynchronously. Use `--wait` to block until complete: From 9eb6bfca2bf3f33c97338566fe3d1985bff4392a Mon Sep 17 00:00:00 2001 From: qin-ctx Date: Fri, 12 Jun 2026 14:53:09 +0800 Subject: [PATCH 7/7] fix(resource): canonicalize watch cancellation targets --- openviking/service/resource_service.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openviking/service/resource_service.py b/openviking/service/resource_service.py index f7401c45fb..d31262b33b 100644 --- a/openviking/service/resource_service.py +++ b/openviking/service/resource_service.py @@ -599,12 +599,12 @@ async def add_resource( logger.warning( f"[ResourceService] Failed to create watch task for {watch_to}: {e}" ) - elif to: + elif target.to: try: - await self._handle_watch_task_cancellation(to_uri=to, ctx=ctx) + await self._handle_watch_task_cancellation(to_uri=target.to, ctx=ctx) except Exception as e: logger.warning( - f"[ResourceService] Failed to cancel watch task for {to}: {e}" + f"[ResourceService] Failed to cancel watch task for {target.to}: {e}" ) if not wait: from openviking.service.task_tracker import get_task_tracker