diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 39f16431a..a2953be53 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3606,6 +3606,7 @@ dependencies = [ "codex-config", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-path-uri", "codex-utils-plugins", "pretty_assertions", "thiserror 2.0.18", diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 490161486..dcdbfa53f 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -209,6 +209,7 @@ "type": "string" }, "path": { + "description": "Absolute path for the root in the selected environment.", "type": "string" }, "type": { @@ -6833,4 +6834,4 @@ } ], "title": "ClientRequest" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index dc5bc6f24..3cecc0e11 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -7042,9 +7042,10 @@ "environmentId": { "type": "string" }, - "path": { - "type": "string" - }, + "path": { + "description": "Absolute path for the root in the selected environment.", + "type": "string" + }, "type": { "enum": [ "environment" @@ -20689,4 +20690,4 @@ }, "title": "CodexAppServerProtocol", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 40a5110a7..b94593a00 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -1169,6 +1169,7 @@ "type": "string" }, "path": { + "description": "Absolute path for the root in the selected environment.", "type": "string" }, "type": { @@ -18467,4 +18468,4 @@ }, "title": "CodexAppServerProtocolV2", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json index 70a877275..e447c644d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json @@ -73,6 +73,7 @@ "type": "string" }, "path": { + "description": "Absolute path for the root in the selected environment.", "type": "string" }, "type": { @@ -387,4 +388,4 @@ }, "title": "ThreadStartParams", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts index 6266101eb..6c2ac9082 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CapabilityRootLocation.ts @@ -5,4 +5,8 @@ /** * Location used to resolve a selected capability root. */ -export type CapabilityRootLocation = { "type": "environment", environmentId: string, path: string, }; +export type CapabilityRootLocation = { "type": "environment", environmentId: string, +/** + * Absolute path for the root in the selected environment. + */ +path: string, }; diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 514b82b72..d9357313b 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -137,7 +137,7 @@ Example with notification opt-out: ## API Overview -- `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. When the request includes a `cwd` and the resolved sandbox is `workspace-write` or full access, app-server also marks that project as trusted in the user `config.toml`. Pass `sessionStartSource: "clear"` when starting a replacement thread after clearing the current session so `SessionStart` hooks receive `source: "clear"` instead of the default `"startup"`. Experimental `runtimeWorkspaceRoots` replaces the thread-scoped runtime workspace roots used to materialize `:workspace_roots`; paths must be absolute. For permissions, prefer experimental `permissions` profile selection by id; the legacy `sandbox` shorthand is still accepted but cannot be combined with `permissions`. Experimental `multiAgentMode` selects the initial thread mode and defaults to `explicitRequestOnly` when omitted; use `none` to keep multi-agent tools available without injecting mode instructions. Experimental `environments` selects the sticky execution environments for turns on the thread; omit it to use the server default, pass `[]` to disable environments, or pass explicit environment ids with per-environment `cwd`. Experimental `selectedCapabilityRoots` selects environment-owned plugin or standalone-skill roots. Skills found below those roots are listed and read through the owning environment. Stdio MCP servers declared by selected plugins are also started in that environment; HTTP MCP declarations remain inactive. +- `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. When the request includes a `cwd` and the resolved sandbox is `workspace-write` or full access, app-server also marks that project as trusted in the user `config.toml`. Pass `sessionStartSource: "clear"` when starting a replacement thread after clearing the current session so `SessionStart` hooks receive `source: "clear"` instead of the default `"startup"`. Experimental `runtimeWorkspaceRoots` replaces the thread-scoped runtime workspace roots used to materialize `:workspace_roots`; paths must be absolute. For permissions, prefer experimental `permissions` profile selection by id; the legacy `sandbox` shorthand is still accepted but cannot be combined with `permissions`. Experimental `multiAgentMode` selects the initial thread mode and defaults to `explicitRequestOnly` when omitted; use `none` to keep multi-agent tools available without injecting mode instructions. Experimental `environments` selects the sticky execution environments for turns on the thread; omit it to use the server default, pass `[]` to disable environments, or pass explicit environment ids with per-environment `cwd`. Experimental `selectedCapabilityRoots` selects environment-owned plugin or standalone-skill roots using environment-native absolute paths. Skills found below those roots are listed and read through the owning environment. Stdio MCP servers declared by selected plugins are also started in that environment; HTTP MCP declarations remain inactive. - `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. Accepts the same permission override rules as `thread/start`. Multi-agent mode restores the last effective mode from rollout history when available; clients can select another mode on the first `turn/start`. - `thread/fork` — fork an existing thread into a new thread id by copying the stored history; if the source thread is currently mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. The returned `thread.forkedFromId` points at the source thread when known. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. Experimental clients can pass `excludeTurns: true` when they plan to page fork history via `thread/turns/list` instead of receiving the full turn array immediately. Accepts the same permission override rules as `thread/start`. - `thread/start`, `thread/resume`, and `thread/fork` responses include the legacy `sandbox` compatibility projection. `instructionSources` lists loaded instruction files using each source environment's native absolute path syntax, including files loaded from remote environments. Experimental clients can read `runtimeWorkspaceRoots` for the thread-scoped runtime roots and `activePermissionProfile` for the named or implicit built-in profile identity/provenance when known. Their experimental `multiAgentMode` field, and the corresponding thread setting, report the thread's current mode. Turn construction separately determines whether that mode is applicable to the selected model and runtime configuration. @@ -267,7 +267,6 @@ Start a fresh thread when you need a new Codex conversation. "location": { "type": "environment", "environmentId": "workspace", - // Opaque to app-server; interpreted in the selected environment. "path": "/opt/cca/plugins/github" } } diff --git a/codex-rs/app-server/tests/suite/v2/executor_mcp.rs b/codex-rs/app-server/tests/suite/v2/executor_mcp.rs index 0eb22a395..299ca81aa 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_mcp.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_mcp.rs @@ -13,6 +13,7 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::UserInput; +use codex_utils_path_uri::PathUri; use core_test_support::responses; use core_test_support::stdio_server_bin; use pretty_assertions::assert_eq; @@ -92,7 +93,7 @@ args = ["exec-server", "--listen", "stdio"] id: "executor-demo@1".to_string(), location: CapabilityRootLocation::Environment { environment_id: EXECUTOR_ID.to_string(), - path: plugin.path().to_string_lossy().into_owned(), + path: PathUri::from_host_native_path(plugin.path())?, }, }]), ) diff --git a/codex-rs/app-server/tests/suite/v2/executor_skills.rs b/codex-rs/app-server/tests/suite/v2/executor_skills.rs index ede5b20fb..a15e285d3 100644 --- a/codex-rs/app-server/tests/suite/v2/executor_skills.rs +++ b/codex-rs/app-server/tests/suite/v2/executor_skills.rs @@ -11,6 +11,7 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::UserInput; +use codex_utils_path_uri::PathUri; use core_test_support::responses; use tempfile::TempDir; use tokio::time::timeout; @@ -90,7 +91,7 @@ stream_max_retries = 0 id: "demo-plugin@1".to_string(), location: CapabilityRootLocation::Environment { environment_id: "local".to_string(), - path: plugin_dir.path().to_string_lossy().into_owned(), + path: PathUri::from_host_native_path(plugin_dir.path())?, }, }]), ..Default::default() diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 14d7a4fe4..0edebe369 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -514,6 +514,11 @@ mod tests { use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_plugin::PluginProvider; use codex_plugin::ResolvedPlugin; + use codex_plugin::manifest::PluginManifest as GenericPluginManifest; + use codex_plugin::manifest::PluginManifestHooks; + use codex_plugin::manifest::PluginManifestInterface; + use codex_plugin::manifest::PluginManifestMcpServers; + use codex_plugin::manifest::PluginManifestPaths; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_utils_absolute_path::AbsolutePathBuf; @@ -779,14 +784,16 @@ mod tests { "composerIcon": "./assets/icon.svg" }"#, ); - let host_manifest = load_plugin_manifest(&plugin_root).expect("host manifest"); + let plugin_root = + AbsolutePathBuf::from_absolute_path_checked(plugin_root).expect("absolute plugin root"); + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); let selected_root = SelectedCapabilityRoot { id: "selected-demo".to_string(), location: CapabilityRootLocation::Environment { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - path: plugin_root.to_string_lossy().into_owned(), + path: plugin_root_uri.clone(), }, }; @@ -795,17 +802,77 @@ mod tests { .await .expect("resolve executor plugin") .expect("plugin descriptor"); - let plugin_root = - AbsolutePathBuf::from_absolute_path_checked(plugin_root).expect("absolute plugin root"); + let manifest_path = plugin_root_uri + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let manifest_contents = + fs::read_to_string(plugin_root.join(".codex-plugin/plugin.json")).expect("manifest"); + let expected_manifest = + super::parse_plugin_manifest_uri(&plugin_root_uri, &manifest_path, &manifest_contents) + .expect("URI manifest"); let expected_plugin = ResolvedPlugin::from_environment( "selected-demo".to_string(), LOCAL_ENVIRONMENT_ID.to_string(), - plugin_root.clone(), - plugin_root.join(".codex-plugin/plugin.json"), - host_manifest, + plugin_root_uri, + manifest_path, + expected_manifest, ) .expect("valid expected descriptor"); assert_eq!(executor_plugin, expected_plugin); } + + #[test] + fn uri_manifest_resolves_resources_below_foreign_root() { + let plugin_root = + PathUri::parse("file:///C:/plugins/demo-plugin").expect("plugin root URI"); + let manifest_path = plugin_root + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let manifest = super::parse_plugin_manifest_uri( + &plugin_root, + &manifest_path, + r#"{ + "name": "demo-plugin", + "skills": "./skills", + "mcpServers": "./.mcp.json", + "apps": "./apps", + "hooks": "./hooks.json", + "interface": { + "displayName": "Demo Plugin", + "composerIcon": "./assets/icon.svg" + } +}"#, + ) + .expect("URI manifest"); + + assert_eq!( + manifest, + GenericPluginManifest { + name: "demo-plugin".to_string(), + version: None, + description: None, + keywords: Vec::new(), + paths: PluginManifestPaths { + skills: vec![plugin_root.join("skills").expect("skills URI")], + mcp_servers: Some(PluginManifestMcpServers::Path( + plugin_root.join(".mcp.json").expect("MCP URI"), + )), + apps: Some(plugin_root.join("apps").expect("apps URI")), + hooks: Some(PluginManifestHooks::Paths(vec![ + plugin_root.join("hooks.json").expect("hooks URI"), + ])), + }, + interface: Some(PluginManifestInterface { + display_name: Some("Demo Plugin".to_string()), + composer_icon: Some( + plugin_root + .join("assets/icon.svg") + .expect("composer icon URI"), + ), + ..PluginManifestInterface::default() + }), + } + ); + } } diff --git a/codex-rs/core-plugins/src/provider.rs b/codex-rs/core-plugins/src/provider.rs index 1a42ad42b..5a371b3d2 100644 --- a/codex-rs/core-plugins/src/provider.rs +++ b/codex-rs/core-plugins/src/provider.rs @@ -1,4 +1,4 @@ -use crate::manifest::parse_plugin_manifest; +use crate::manifest::parse_plugin_manifest_uri; use codex_exec_server::EnvironmentManager; use codex_exec_server::ExecutorFileSystem; use codex_plugin::PluginProvider; @@ -6,23 +6,16 @@ use codex_plugin::ResolvedPlugin; use codex_plugin::ResolvedPluginError; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; +use codex_utils_path_uri::PathUriParseError; use codex_utils_plugins::DISCOVERABLE_PLUGIN_MANIFEST_PATHS; use std::io; -use std::path::PathBuf; use std::sync::Arc; use thiserror::Error; /// Failure to resolve an environment-owned capability root as a plugin package. #[derive(Debug, Error)] pub enum ExecutorPluginProviderError { - #[error("selected capability root `{root_id}` has invalid path `{path}`: {message}")] - InvalidRootPath { - root_id: String, - path: String, - message: String, - }, #[error( "selected capability root `{root_id}` references unavailable environment `{environment_id}`" )] @@ -33,33 +26,40 @@ pub enum ExecutorPluginProviderError { #[error("failed to inspect selected capability root `{root_id}` at {path}: {source}")] InspectRoot { root_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: io::Error, }, #[error("selected capability root `{root_id}` path {path} is not a directory")] - RootNotDirectory { + RootNotDirectory { root_id: String, path: PathUri }, + #[error( + "failed to resolve plugin manifest path `{relative_path}` below selected capability root `{root_id}` at {root}: {source}" + )] + InvalidManifestPath { root_id: String, - path: AbsolutePathBuf, + root: PathUri, + relative_path: &'static str, + #[source] + source: PathUriParseError, }, #[error("failed to inspect plugin manifest for `{root_id}` at {path}: {source}")] InspectManifest { root_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: io::Error, }, #[error("failed to read plugin manifest for `{root_id}` at {path}: {source}")] ReadManifest { root_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: io::Error, }, #[error("failed to parse plugin manifest for `{root_id}` at {path}: {source}")] ParseManifest { root_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: serde_json::Error, }, @@ -110,7 +110,7 @@ impl ExecutorPluginProvider { selected_root: &SelectedCapabilityRoot, ) -> Result, ExecutorPluginProviderError> { let root_id = &selected_root.id; - let plugin_root = selected_plugin_root(selected_root)?; + let plugin_root = selected_plugin_root(selected_root); let CapabilityRootLocation::Environment { environment_id, .. } = &selected_root.location; let environment = self .environment_manager @@ -142,38 +142,20 @@ impl PluginProvider for ExecutorPluginProvider { } } -fn selected_plugin_root( - selected_root: &SelectedCapabilityRoot, -) -> Result { - let root_id = &selected_root.id; +fn selected_plugin_root(selected_root: &SelectedCapabilityRoot) -> PathUri { let CapabilityRootLocation::Environment { path, .. } = &selected_root.location; - let plugin_root = PathBuf::from(path); - if !plugin_root.is_absolute() { - return Err(ExecutorPluginProviderError::InvalidRootPath { - root_id: root_id.clone(), - path: path.clone(), - message: "executor path must be absolute".to_string(), - }); - } - AbsolutePathBuf::from_absolute_path_checked(plugin_root).map_err(|err| { - ExecutorPluginProviderError::InvalidRootPath { - root_id: root_id.clone(), - path: path.clone(), - message: err.to_string(), - } - }) + path.clone() } async fn resolve_plugin_root( selected_root: &SelectedCapabilityRoot, - plugin_root: AbsolutePathBuf, + plugin_root: PathUri, file_system: &dyn ExecutorFileSystem, ) -> Result, ExecutorPluginProviderError> { let root_id = &selected_root.id; let CapabilityRootLocation::Environment { environment_id, .. } = &selected_root.location; - let root_uri = PathUri::from_abs_path(&plugin_root); let root_metadata = file_system - .get_metadata(&root_uri, /*sandbox*/ None) + .get_metadata(&plugin_root, /*sandbox*/ None) .await .map_err(|source| ExecutorPluginProviderError::InspectRoot { root_id: root_id.clone(), @@ -189,14 +171,20 @@ async fn resolve_plugin_root( let mut manifest_path = None; for relative_path in DISCOVERABLE_PLUGIN_MANIFEST_PATHS { - let candidate = plugin_root.join(relative_path); - let candidate_uri = PathUri::from_abs_path(&candidate); + let candidate_uri = plugin_root.join(relative_path).map_err(|source| { + ExecutorPluginProviderError::InvalidManifestPath { + root_id: root_id.clone(), + root: plugin_root.clone(), + relative_path, + source, + } + })?; match file_system .get_metadata(&candidate_uri, /*sandbox*/ None) .await { Ok(metadata) if metadata.is_file => { - manifest_path = Some((candidate, candidate_uri)); + manifest_path = Some(candidate_uri); break; } Ok(_) => {} @@ -204,13 +192,13 @@ async fn resolve_plugin_root( Err(source) => { return Err(ExecutorPluginProviderError::InspectManifest { root_id: root_id.clone(), - path: candidate, + path: candidate_uri, source, }); } } } - let Some((manifest_path, manifest_uri)) = manifest_path else { + let Some(manifest_uri) = manifest_path else { return Ok(None); }; let contents = file_system @@ -218,14 +206,14 @@ async fn resolve_plugin_root( .await .map_err(|source| ExecutorPluginProviderError::ReadManifest { root_id: root_id.clone(), - path: manifest_path.clone(), + path: manifest_uri.clone(), source, })?; let manifest = - parse_plugin_manifest(&plugin_root, &manifest_path, &contents).map_err(|source| { + parse_plugin_manifest_uri(&plugin_root, &manifest_uri, &contents).map_err(|source| { ExecutorPluginProviderError::ParseManifest { root_id: root_id.clone(), - path: manifest_path.clone(), + path: manifest_uri.clone(), source, } })?; @@ -234,7 +222,7 @@ async fn resolve_plugin_root( root_id.clone(), environment_id.clone(), plugin_root, - manifest_path, + manifest_uri, manifest, ) .map_err(|source| ExecutorPluginProviderError::ConstructDescriptor { diff --git a/codex-rs/core-plugins/src/provider_tests.rs b/codex-rs/core-plugins/src/provider_tests.rs index 3f92a47f0..e35a09494 100644 --- a/codex-rs/core-plugins/src/provider_tests.rs +++ b/codex-rs/core-plugins/src/provider_tests.rs @@ -1,7 +1,7 @@ use super::ExecutorPluginProvider; use super::ExecutorPluginProviderError; use super::resolve_plugin_root; -use crate::manifest::parse_plugin_manifest; +use crate::manifest::parse_plugin_manifest_uri; use codex_exec_server::CopyOptions; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::EnvironmentManager; @@ -18,7 +18,6 @@ use codex_plugin::PluginProvider; use codex_plugin::ResolvedPlugin; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::fs; @@ -43,13 +42,13 @@ const MANIFEST_CONTENTS: &str = r#"{ #[derive(Debug, PartialEq, Eq)] enum FileSystemCall { - Metadata(AbsolutePathBuf), - Read(AbsolutePathBuf), + Metadata(PathUri), + Read(PathUri), } struct SyntheticPluginFileSystem { - plugin_root: AbsolutePathBuf, - manifest_path: AbsolutePathBuf, + plugin_root: PathUri, + manifest_path: PathUri, calls: Mutex>, } @@ -77,12 +76,11 @@ impl ExecutorFileSystem for SyntheticPluginFileSystem { _sandbox: Option<&'a FileSystemSandboxContext>, ) -> ExecutorFileSystemFuture<'a, Vec> { Box::pin(async move { - let path = path.to_abs_path()?; self.calls .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) .push(FileSystemCall::Read(path.clone())); - if path == self.manifest_path { + if path == &self.manifest_path { Ok(MANIFEST_CONTENTS.as_bytes().to_vec()) } else { Err(io::Error::new(io::ErrorKind::NotFound, "not found")) @@ -122,14 +120,13 @@ impl ExecutorFileSystem for SyntheticPluginFileSystem { _sandbox: Option<&'a FileSystemSandboxContext>, ) -> ExecutorFileSystemFuture<'a, FileMetadata> { Box::pin(async move { - let path = path.to_abs_path()?; self.calls .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) .push(FileSystemCall::Metadata(path.clone())); - let (is_directory, is_file) = if path == self.plugin_root { + let (is_directory, is_file) = if path == &self.plugin_root { (true, false) - } else if path == self.manifest_path { + } else if path == &self.manifest_path { (false, true) } else { return Err(io::Error::new(io::ErrorKind::NotFound, "not found")); @@ -185,7 +182,17 @@ fn selected_root(id: &str, environment_id: &str, path: &Path) -> SelectedCapabil id: id.to_string(), location: CapabilityRootLocation::Environment { environment_id: environment_id.to_string(), - path: path.to_string_lossy().into_owned(), + path: PathUri::from_host_native_path(path).expect("path URI"), + }, + } +} + +fn selected_root_uri(id: &str, environment_id: &str, path: PathUri) -> SelectedCapabilityRoot { + SelectedCapabilityRoot { + id: id.to_string(), + location: CapabilityRootLocation::Environment { + environment_id: environment_id.to_string(), + path, }, } } @@ -195,22 +202,20 @@ async fn plugin_root_resolution_uses_supplied_executor_file_system() { let temp_dir = tempdir().expect("tempdir"); let plugin_root = temp_dir.path().join("executor-only-plugin"); assert!(!plugin_root.exists()); - let plugin_root = - AbsolutePathBuf::from_absolute_path_checked(plugin_root).expect("absolute plugin root"); - let manifest_path = plugin_root.join(".codex-plugin/plugin.json"); - let parsed_manifest = parse_plugin_manifest( - plugin_root.as_path(), - manifest_path.as_path(), - MANIFEST_CONTENTS, - ) - .expect("parse manifest"); + let plugin_root = PathUri::from_host_native_path(&plugin_root).expect("plugin root URI"); + let manifest_path = plugin_root + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let parsed_manifest = + parse_plugin_manifest_uri(&plugin_root, &manifest_path, MANIFEST_CONTENTS) + .expect("parse manifest"); let file_system = SyntheticPluginFileSystem { plugin_root: plugin_root.clone(), manifest_path: manifest_path.clone(), calls: Mutex::new(Vec::new()), }; let resolved = resolve_plugin_root( - &selected_root("selected-demo", "executor-test", plugin_root.as_path()), + &selected_root_uri("selected-demo", "executor-test", plugin_root.clone()), plugin_root.clone(), &file_system, ) @@ -243,6 +248,51 @@ async fn plugin_root_resolution_uses_supplied_executor_file_system() { ); } +#[tokio::test] +async fn plugin_root_resolution_accepts_foreign_executor_file_uri() { + let plugin_root = PathUri::parse("file:///C:/plugins/foo").expect("Windows plugin root URI"); + let manifest_path = plugin_root + .join(".codex-plugin/plugin.json") + .expect("manifest URI"); + let parsed_manifest = + parse_plugin_manifest_uri(&plugin_root, &manifest_path, MANIFEST_CONTENTS) + .expect("parse manifest"); + let file_system = SyntheticPluginFileSystem { + plugin_root: plugin_root.clone(), + manifest_path: manifest_path.clone(), + calls: Mutex::new(Vec::new()), + }; + let selected_root = selected_root_uri("selected-demo", "executor-test", plugin_root.clone()); + let resolved = resolve_plugin_root(&selected_root, plugin_root.clone(), &file_system) + .await + .expect("resolve executor plugin"); + + assert_eq!( + resolved, + Some( + ResolvedPlugin::from_environment( + "selected-demo".to_string(), + "executor-test".to_string(), + plugin_root.clone(), + manifest_path.clone(), + parsed_manifest, + ) + .expect("valid expected descriptor") + ) + ); + assert_eq!( + *file_system + .calls + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner), + vec![ + FileSystemCall::Metadata(plugin_root), + FileSystemCall::Metadata(manifest_path.clone()), + FileSystemCall::Read(manifest_path), + ] + ); +} + #[tokio::test] async fn standalone_capability_root_is_not_a_plugin() { let temp_dir = tempdir().expect("tempdir"); @@ -292,8 +342,8 @@ async fn malformed_preferred_manifest_does_not_fall_through_to_alternate() { MANIFEST_CONTENTS, ); let expected_path = - AbsolutePathBuf::from_absolute_path_checked(plugin_root.join(".codex-plugin/plugin.json")) - .expect("absolute manifest path"); + PathUri::from_host_native_path(plugin_root.join(".codex-plugin/plugin.json")) + .expect("manifest URI"); let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); let err = provider @@ -318,25 +368,3 @@ async fn malformed_preferred_manifest_does_not_fall_through_to_alternate() { ("selected-demo".to_string(), expected_path) ); } - -#[tokio::test] -async fn executor_root_must_be_an_explicit_absolute_path() { - let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); - let selected_root = SelectedCapabilityRoot { - id: "selected-demo".to_string(), - location: CapabilityRootLocation::Environment { - environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - path: "~/plugins/demo".to_string(), - }, - }; - - let err = provider - .resolve(&selected_root) - .await - .expect_err("home-relative executor path should fail"); - - assert_eq!( - err.to_string(), - "selected capability root `selected-demo` has invalid path `~/plugins/demo`: executor path must be absolute" - ); -} diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index ea9879bbd..990c5ca5d 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -453,7 +453,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() id: id.to_string(), location: CapabilityRootLocation::Environment { environment_id: environment_id.to_string(), - path: format!("/plugins/{id}"), + path: PathUri::parse(&format!("file:///plugins/{id}")).expect("plugin root URI"), }, }]); init diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider.rs b/codex-rs/ext/mcp/src/executor_plugin/provider.rs index e9d5f803d..d3f264fcc 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider.rs @@ -8,9 +8,10 @@ use codex_plugin::PluginResourceLocator; use codex_plugin::ResolvedPlugin; use codex_plugin::ResolvedPluginLocation; use codex_plugin::manifest::PluginManifestMcpServers; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; +use codex_utils_path_uri::PathUriParseError; use std::io; +use std::path::PathBuf; use thiserror::Error; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; @@ -25,14 +26,24 @@ pub(super) enum ExecutorPluginMcpProviderError { #[error("failed to read MCP config for selected plugin `{plugin_id}` at `{path}`: {source}")] ReadConfig { plugin_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: io::Error, }, + #[error( + "failed to resolve MCP config path `{relative_path}` below selected plugin `{plugin_id}` at `{root}`: {source}" + )] + InvalidConfigPath { + plugin_id: String, + root: PathUri, + relative_path: &'static str, + #[source] + source: PathUriParseError, + }, #[error("failed to parse MCP config for selected plugin `{plugin_id}` at `{path}`: {source}")] ParseConfig { plugin_id: String, - path: AbsolutePathBuf, + path: PathUri, #[source] source: serde_json::Error, }, @@ -52,7 +63,7 @@ impl ExecutorPluginMcpProvider { async fn load_from_file_system( plugin: &ResolvedPlugin, - plugin_root: &AbsolutePathBuf, + plugin_root: &PathUri, file_system: &dyn ExecutorFileSystem, ) -> Result, ExecutorPluginMcpProviderError> { let ResolvedPluginLocation::Environment { environment_id, .. } = plugin.location(); @@ -61,10 +72,9 @@ async fn load_from_file_system( Some(PluginManifestMcpServers::Path(PluginResourceLocator::Environment { path, .. })) => { - let config_uri = PathUri::from_abs_path(path); ( file_system - .read_file_text(&config_uri, /*sandbox*/ None) + .read_file_text(path, /*sandbox*/ None) .await .map_err(|source| ExecutorPluginMcpProviderError::ReadConfig { plugin_id: plugin_id.to_string(), @@ -74,15 +84,21 @@ async fn load_from_file_system( path.clone(), ) } - Some(PluginManifestMcpServers::Object(object_config)) => ( - object_config.clone(), - plugin_root.join(".codex-plugin/plugin.json"), - ), + Some(PluginManifestMcpServers::Object(object_config)) => { + let PluginResourceLocator::Environment { path, .. } = plugin.manifest_path(); + (object_config.clone(), path.clone()) + } None => { - let config_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE); - let config_uri = PathUri::from_abs_path(&config_path); + let config_path = plugin_root + .join(DEFAULT_MCP_CONFIG_FILE) + .map_err(|source| ExecutorPluginMcpProviderError::InvalidConfigPath { + plugin_id: plugin_id.to_string(), + root: plugin_root.clone(), + relative_path: DEFAULT_MCP_CONFIG_FILE, + source, + })?; let contents = match file_system - .read_file_text(&config_uri, /*sandbox*/ None) + .read_file_text(&config_path, /*sandbox*/ None) .await { Ok(contents) => contents, @@ -100,8 +116,9 @@ async fn load_from_file_system( (contents, config_path) } }; + let plugin_root_path = PathBuf::from(plugin_root.inferred_native_path_string()); let parsed = parse_plugin_mcp_config( - plugin_root.as_path(), + plugin_root_path.as_path(), &contents, PluginMcpServerPlacement::Environment { environment_id }, ) diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs index 07d662185..a007acaac 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs @@ -156,7 +156,8 @@ async fn reads_declared_config_only_through_executor_file_system() { reads: Mutex::new(Vec::new()), }; - let servers = load_from_file_system(&plugin, &plugin_root, &file_system) + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system) .await .expect("load executor MCP config"); @@ -210,7 +211,8 @@ async fn reads_manifest_object_config_without_executor_file_system_access() { reads: Mutex::new(Vec::new()), }; - let servers = load_from_file_system(&plugin, &plugin_root, &file_system) + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system) .await .expect("load manifest object executor MCP config"); @@ -259,7 +261,8 @@ async fn missing_default_config_is_empty() { reads: Mutex::new(Vec::new()), }; - let servers = load_from_file_system(&plugin, &plugin_root, &file_system) + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system) .await .expect("missing default config should be ignored"); @@ -283,7 +286,8 @@ async fn malformed_declared_config_is_an_error() { reads: Mutex::new(Vec::new()), }; - let err = load_from_file_system(&plugin, &plugin_root, &file_system) + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let err = load_from_file_system(&plugin, &plugin_root_uri, &file_system) .await .expect_err("malformed declared config should fail"); @@ -297,20 +301,70 @@ async fn malformed_declared_config_is_an_error() { }; assert_eq!( (plugin_id, path), - ("selected-root".to_string(), config_path.clone()) + ( + "selected-root".to_string(), + PathUri::from_abs_path(&config_path) + ) ); assert_eq!(reads(&file_system), vec![config_path]); } +#[tokio::test] +async fn malformed_manifest_object_config_reports_actual_manifest_path() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let plugin_root = AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("plugin")) + .expect("absolute plugin root"); + let plugin = resolved_plugin( + &plugin_root, + Some(PluginManifestMcpServers::Object("{not-json".to_string())), + ); + let file_system = SyntheticExecutorFileSystem { + config_path: plugin_root.join(DEFAULT_MCP_CONFIG_FILE), + config_contents: None, + reads: Mutex::new(Vec::new()), + }; + + let plugin_root_uri = PathUri::from_abs_path(&plugin_root); + let err = load_from_file_system(&plugin, &plugin_root_uri, &file_system) + .await + .expect_err("malformed manifest object config should fail"); + + let ExecutorPluginMcpProviderError::ParseConfig { + plugin_id, + path, + source: _, + } = err + else { + panic!("expected parse error"); + }; + assert_eq!( + (plugin_id, path), + ( + "selected-root".to_string(), + PathUri::from_abs_path(&plugin_root.join(".claude-plugin/plugin.json")) + ) + ); + assert_eq!(reads(&file_system), Vec::new()); +} + fn resolved_plugin( plugin_root: &AbsolutePathBuf, mcp_servers: Option>, ) -> ResolvedPlugin { + let plugin_root_uri = PathUri::from_abs_path(plugin_root); + let mcp_servers = mcp_servers.map(|mcp_servers| match mcp_servers { + PluginManifestMcpServers::Path(path) => { + PluginManifestMcpServers::Path(PathUri::from_abs_path(&path)) + } + PluginManifestMcpServers::Object(config) => PluginManifestMcpServers::Object(config), + }); ResolvedPlugin::from_environment( "selected-root".to_string(), "executor-test".to_string(), - plugin_root.clone(), - plugin_root.join(".codex-plugin/plugin.json"), + plugin_root_uri.clone(), + plugin_root_uri + .join(".claude-plugin/plugin.json") + .expect("manifest URI"), PluginManifest { name: "demo-plugin".to_string(), version: None, diff --git a/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs b/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs index 8820b3510..7bdbc83d6 100644 --- a/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs +++ b/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs @@ -9,6 +9,7 @@ use codex_extension_api::McpServerContribution; use codex_extension_api::McpServerContributionContext; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::sync::Arc; @@ -59,7 +60,7 @@ command = "expected-command" .build() .await?; - let contributions = selected_plugin_contributions(&config, plugin_root.path()).await; + let contributions = selected_plugin_contributions(&config, plugin_root.path()).await?; assert_eq!( contributions, @@ -93,7 +94,7 @@ command = "expected-command" async fn selected_plugin_contributions( config: &Config, plugin_root: &std::path::Path, -) -> Vec { +) -> Result, Box> { let mut builder = ExtensionRegistryBuilder::new(); codex_mcp_extension::install_executor_plugins( &mut builder, @@ -105,12 +106,12 @@ async fn selected_plugin_contributions( id: "selected-root".to_string(), location: CapabilityRootLocation::Environment { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - path: plugin_root.to_string_lossy().into_owned(), + path: PathUri::from_host_native_path(plugin_root)?, }, }]); codex_mcp_extension::initialize_executor_plugin_thread_data(&mut thread_init); - registry.mcp_server_contributors()[0] + Ok(registry.mcp_server_contributors()[0] .contribute(McpServerContributionContext::for_thread( config, &thread_init, @@ -135,5 +136,5 @@ async fn selected_plugin_contributions( panic!("expected selected plugin contribution") } }) - .collect() + .collect()) } diff --git a/codex-rs/ext/skills/src/provider/executor.rs b/codex-rs/ext/skills/src/provider/executor.rs index 75234dfbe..273a1614b 100644 --- a/codex-rs/ext/skills/src/provider/executor.rs +++ b/codex-rs/ext/skills/src/provider/executor.rs @@ -1,4 +1,3 @@ -use std::path::PathBuf; use std::sync::Arc; use codex_core_skills::SkillMetadata; @@ -197,13 +196,6 @@ fn catalog_entry_from_skill( entry } -fn executor_absolute_path(path: &str) -> std::io::Result { - let path = PathBuf::from(path); - if !path.is_absolute() { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "executor path must be absolute", - )); - } - AbsolutePathBuf::from_absolute_path_checked(path) +fn executor_absolute_path(path: &PathUri) -> std::io::Result { + path.to_abs_path() } diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index e4b09e96a..8c5160e2c 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -222,7 +222,6 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() { #[tokio::test] async fn selected_root_id_distinguishes_identical_executor_paths() { let test_root = create_local_skill_root("root-identity").expect("create local skill root"); - let root_path = test_root.to_string_lossy().into_owned(); let canonical_root = AbsolutePathBuf::from_absolute_path_checked(&test_root) .expect("absolute skill root") .canonicalize() @@ -242,7 +241,7 @@ async fn selected_root_id_distinguishes_identical_executor_paths() { id: id.to_string(), location: CapabilityRootLocation::Environment { environment_id: "local".to_string(), - path: root_path.clone(), + path: PathUri::from_host_native_path(&test_root).expect("skill root URI"), }, }) .collect(), diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index 8b0255834..761dc34d7 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -48,6 +48,7 @@ use codex_skills_extension::provider::SkillProviderFuture; use codex_skills_extension::provider::SkillReadRequest; use codex_skills_extension::provider::SkillSearchRequest; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; type TestResult = Result<(), Box>; @@ -171,7 +172,7 @@ async fn selected_executor_catalog_is_context_and_selected_entrypoint_is_turn_in id: "lint-fix".to_string(), location: CapabilityRootLocation::Environment { environment_id: "env-1".to_string(), - path: "/skills/lint-fix".to_string(), + path: PathUri::parse("file:///skills/lint-fix").expect("skill root URI"), }, }]); let session_source = SessionSource::Cli; @@ -494,7 +495,7 @@ async fn root_qualified_locator_selects_only_the_matching_executor_skill() -> Te id: id.to_string(), location: CapabilityRootLocation::Environment { environment_id: "env-1".to_string(), - path: path.to_string(), + path: PathUri::parse(&format!("file://{path}")).expect("skill root URI"), }, }) .collect::>(), diff --git a/codex-rs/plugin/Cargo.toml b/codex-rs/plugin/Cargo.toml index 77116dfa3..3e55f55c3 100644 --- a/codex-rs/plugin/Cargo.toml +++ b/codex-rs/plugin/Cargo.toml @@ -16,6 +16,7 @@ workspace = true codex-config = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-plugins = { workspace = true } thiserror = { workspace = true } diff --git a/codex-rs/plugin/src/provider.rs b/codex-rs/plugin/src/provider.rs index d2d8cc7c8..6dd81db8a 100644 --- a/codex-rs/plugin/src/provider.rs +++ b/codex-rs/plugin/src/provider.rs @@ -1,6 +1,6 @@ use crate::manifest::PluginManifest; use codex_protocol::capabilities::SelectedCapabilityRoot; -use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::error::Error as StdError; use std::future::Future; use thiserror::Error; @@ -11,8 +11,8 @@ pub enum PluginResourceLocator { Environment { /// Environment whose filesystem owns the resource. environment_id: String, - /// Absolute resource path within that filesystem. - path: AbsolutePathBuf, + /// Resource URI within that filesystem. + path: PathUri, }, } @@ -22,8 +22,8 @@ pub enum ResolvedPluginLocation { Environment { /// Environment whose filesystem owns the package. environment_id: String, - /// Absolute package root within that filesystem. - root: AbsolutePathBuf, + /// Package root URI within that filesystem. + root: PathUri, }, } @@ -40,10 +40,7 @@ pub struct ResolvedPlugin { #[derive(Clone, Debug, Error, PartialEq, Eq)] pub enum ResolvedPluginError { #[error("plugin resource path `{path}` is outside package root `{root}`")] - ResourceOutsideRoot { - root: AbsolutePathBuf, - path: AbsolutePathBuf, - }, + ResourceOutsideRoot { root: PathUri, path: PathUri }, } impl ResolvedPlugin { @@ -51,9 +48,9 @@ impl ResolvedPlugin { pub fn from_environment( selected_root_id: String, environment_id: String, - root: AbsolutePathBuf, - manifest_path: AbsolutePathBuf, - manifest: PluginManifest, + root: PathUri, + manifest_path: PathUri, + manifest: PluginManifest, ) -> Result { let manifest_path = environment_resource(&environment_id, &root, manifest_path)?; let manifest = manifest @@ -92,10 +89,10 @@ impl ResolvedPlugin { fn environment_resource( environment_id: &str, - root: &AbsolutePathBuf, - path: AbsolutePathBuf, + root: &PathUri, + path: PathUri, ) -> Result { - if !path.as_path().starts_with(root.as_path()) { + if !path.starts_with(root) { return Err(ResolvedPluginError::ResourceOutsideRoot { root: root.clone(), path, diff --git a/codex-rs/plugin/src/provider_tests.rs b/codex-rs/plugin/src/provider_tests.rs index 99bc3c208..7f9e70d33 100644 --- a/codex-rs/plugin/src/provider_tests.rs +++ b/codex-rs/plugin/src/provider_tests.rs @@ -7,22 +7,28 @@ use crate::manifest::PluginManifestInterface; use crate::manifest::PluginManifestMcpServers; use crate::manifest::PluginManifestPaths; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; fn absolute(path: impl AsRef) -> AbsolutePathBuf { AbsolutePathBuf::from_absolute_path_checked(path.as_ref()).expect("absolute test path") } -fn resource(environment_id: &str, path: AbsolutePathBuf) -> PluginResourceLocator { +fn path_uri(path: &AbsolutePathBuf) -> PathUri { + PathUri::from_abs_path(path) +} + +fn resource(environment_id: &str, path: &AbsolutePathBuf) -> PluginResourceLocator { PluginResourceLocator::Environment { environment_id: environment_id.to_string(), - path, + path: path_uri(path), } } #[test] fn environment_descriptor_binds_every_manifest_resource() { let root = absolute(std::env::current_dir().expect("cwd").join("plugin-root")); + let root_uri = path_uri(&root); let manifest_path = root.join(".codex-plugin/plugin.json"); let skills = root.join("skills"); let mcp_servers = root.join(".mcp.json"); @@ -37,15 +43,15 @@ fn environment_descriptor_binds_every_manifest_resource() { description: None, keywords: Vec::new(), paths: PluginManifestPaths { - skills: vec![skills.clone()], - mcp_servers: Some(PluginManifestMcpServers::Path(mcp_servers.clone())), - apps: Some(apps.clone()), - hooks: Some(PluginManifestHooks::Paths(vec![hooks.clone()])), + skills: vec![path_uri(&skills)], + mcp_servers: Some(PluginManifestMcpServers::Path(path_uri(&mcp_servers))), + apps: Some(path_uri(&apps)), + hooks: Some(PluginManifestHooks::Paths(vec![path_uri(&hooks)])), }, interface: Some(PluginManifestInterface { - composer_icon: Some(composer_icon.clone()), - logo: Some(logo.clone()), - screenshots: vec![screenshot.clone()], + composer_icon: Some(path_uri(&composer_icon)), + logo: Some(path_uri(&logo)), + screenshots: vec![path_uri(&screenshot)], ..PluginManifestInterface::default() }), }; @@ -53,15 +59,15 @@ fn environment_descriptor_binds_every_manifest_resource() { let plugin = ResolvedPlugin::from_environment( "selected-demo".to_string(), "executor-1".to_string(), - root, - manifest_path.clone(), + root_uri, + path_uri(&manifest_path), manifest, ) .expect("valid descriptor"); assert_eq!( plugin.manifest_path(), - &resource("executor-1", manifest_path) + &resource("executor-1", &manifest_path) ); assert_eq!( plugin.manifest(), @@ -71,21 +77,21 @@ fn environment_descriptor_binds_every_manifest_resource() { description: None, keywords: Vec::new(), paths: PluginManifestPaths { - skills: vec![resource("executor-1", skills)], + skills: vec![resource("executor-1", &skills)], mcp_servers: Some(PluginManifestMcpServers::Path(resource( "executor-1", - mcp_servers, + &mcp_servers, ))), - apps: Some(resource("executor-1", apps)), + apps: Some(resource("executor-1", &apps)), hooks: Some(PluginManifestHooks::Paths(vec![resource( "executor-1", - hooks, + &hooks )])), }, interface: Some(PluginManifestInterface { - composer_icon: Some(resource("executor-1", composer_icon)), - logo: Some(resource("executor-1", logo)), - screenshots: vec![resource("executor-1", screenshot)], + composer_icon: Some(resource("executor-1", &composer_icon)), + logo: Some(resource("executor-1", &logo)), + screenshots: vec![resource("executor-1", &screenshot)], ..PluginManifestInterface::default() }), } @@ -104,7 +110,7 @@ fn environment_descriptor_rejects_resources_outside_package_root() { keywords: Vec::new(), paths: PluginManifestPaths { skills: Vec::new(), - mcp_servers: Some(PluginManifestMcpServers::Path(outside.clone())), + mcp_servers: Some(PluginManifestMcpServers::Path(path_uri(&outside))), apps: None, hooks: None, }, @@ -114,8 +120,8 @@ fn environment_descriptor_rejects_resources_outside_package_root() { let err = ResolvedPlugin::from_environment( "selected-demo".to_string(), "executor-1".to_string(), - root.clone(), - root.join(".codex-plugin/plugin.json"), + path_uri(&root), + path_uri(&root.join(".codex-plugin/plugin.json")), manifest, ) .expect_err("outside resource should fail"); @@ -123,8 +129,8 @@ fn environment_descriptor_rejects_resources_outside_package_root() { assert_eq!( err, ResolvedPluginError::ResourceOutsideRoot { - root, - path: outside, + root: path_uri(&root), + path: path_uri(&outside), } ); } diff --git a/codex-rs/protocol/src/capabilities.rs b/codex-rs/protocol/src/capabilities.rs index cfdead7b2..f9fe8dcc6 100644 --- a/codex-rs/protocol/src/capabilities.rs +++ b/codex-rs/protocol/src/capabilities.rs @@ -1,3 +1,5 @@ +use codex_utils_path_uri::LegacyAppPathString; +use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -25,6 +27,25 @@ pub enum CapabilityRootLocation { #[serde(rename = "environmentId")] #[ts(rename = "environmentId")] environment_id: String, - path: String, + /// Absolute path for the root in the selected environment. + #[serde(deserialize_with = "deserialize_path_uri_from_api_path")] + #[schemars(with = "String")] + #[ts(type = "string")] + path: PathUri, }, } + +fn deserialize_path_uri_from_api_path<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let path = LegacyAppPathString::deserialize(deserializer)?; + if let Ok(path_uri) = PathUri::parse(path.as_str()) { + return Ok(path_uri); + } + path.try_into().map_err(serde::de::Error::custom) +} + +#[cfg(test)] +#[path = "capabilities_tests.rs"] +mod tests; diff --git a/codex-rs/protocol/src/capabilities_tests.rs b/codex-rs/protocol/src/capabilities_tests.rs new file mode 100644 index 000000000..05f713ee8 --- /dev/null +++ b/codex-rs/protocol/src/capabilities_tests.rs @@ -0,0 +1,30 @@ +use super::CapabilityRootLocation; +use super::SelectedCapabilityRoot; +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; + +#[test] +fn environment_capability_root_accepts_foreign_file_uri() { + let selected_root = serde_json::from_str::( + r#"{ + "id": "selected-demo", + "location": { + "type": "environment", + "environmentId": "executor-test", + "path": "file:///C:/plugins/demo" + } + }"#, + ) + .expect("file URI should deserialize"); + + assert_eq!( + selected_root, + SelectedCapabilityRoot { + id: "selected-demo".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: "executor-test".to_string(), + path: PathUri::parse("file:///C:/plugins/demo").expect("path URI"), + }, + } + ); +} diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index bbbdaa16c..7b84beccb 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -246,10 +246,10 @@ impl PathUri { /// Returns true when this URI is lexically equal to or below `base`. /// /// Containment is computed using URI authority and path-segment boundaries, - /// without consulting the host filesystem. Percent-encoded path separators - /// fail closed because native path conversion may interpret them as segment - /// boundaries. Opaque fallback URIs created by [`Self::from_abs_path`] only - /// contain themselves. + /// without consulting the host filesystem. Percent-encoded native path + /// separators fail closed because native path conversion may interpret them + /// as segment boundaries. Opaque fallback URIs created by + /// [`Self::from_abs_path`] only contain themselves. pub fn starts_with(&self, base: &Self) -> bool { if self == base { return true;