mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Use PathUri for skill identity and path matching (#51482)
## Why Skill selection and disabled-path matching need to recognize equivalent Windows paths across case and separator differences while preserving literal spaces, `%`, and `#` in native filenames. ## What changed - Carry `PathUri` through skill metadata, discovery mappings, filesystem lookups, disabled-path sets, and invocation analytics. - Match linked skill mentions and TUI selections by parsed path identity. - Use `LegacyAppPathString` for app-server skill paths and regenerate schemas and SDK types, preserving native path strings on the wire. - Preserve native catalog display paths and ordering. Keep host filesystem probes and configuration writes restricted to host-compatible paths. ## Testing Add regression tests for Windows path identity and disabled-skill matching, literal filename characters, catalog resource reads, native path ordering, and foreign-path analytics normalization. GitOrigin-RevId: c212916dc9fef0e7616542726e107f0d6a313267
This commit is contained in:
committed by
copyberry
parent
e79c498b5e
commit
c9870d0157
Generated
+1
@@ -2012,6 +2012,7 @@ dependencies = [
|
||||
"codex-protocol",
|
||||
"codex-state",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-path-uri",
|
||||
"os_info",
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
|
||||
@@ -21,6 +21,7 @@ codex-model-provider = { workspace = true }
|
||||
codex-plugin = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-state = { workspace = true }
|
||||
codex-utils-path-uri = { workspace = true }
|
||||
os_info = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
|
||||
@@ -33,6 +33,7 @@ use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::ThreadSource;
|
||||
use codex_protocol::protocol::TokenUsage;
|
||||
use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use serde::Serialize;
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::PathBuf;
|
||||
@@ -387,7 +388,7 @@ pub struct SkillInvocation {
|
||||
#[derive(Clone, Debug)]
|
||||
pub enum SkillInvocationLocation {
|
||||
Host {
|
||||
path: PathBuf,
|
||||
path: PathUri,
|
||||
scope: SkillScope,
|
||||
},
|
||||
Resource {
|
||||
|
||||
@@ -172,6 +172,7 @@ use codex_protocol::protocol::ThreadSource;
|
||||
use codex_protocol::protocol::TokenUsage;
|
||||
use codex_protocol::request_permissions::PermissionGrantScope as CorePermissionGrantScope;
|
||||
use codex_protocol::request_permissions::RequestPermissionsResponse as CoreRequestPermissionsResponse;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use sha1::Digest;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
@@ -1356,7 +1357,10 @@ impl AnalyticsReducer {
|
||||
SkillScope::System => "system",
|
||||
SkillScope::Admin => "admin",
|
||||
};
|
||||
let repo_root = get_git_repo_root(path.as_path());
|
||||
let repo_root = path
|
||||
.to_abs_path()
|
||||
.ok()
|
||||
.and_then(|path| get_git_repo_root(path.as_path()));
|
||||
let repo_url = if let Some(root) = repo_root.as_ref() {
|
||||
get_git_origin_url(root).await
|
||||
} else {
|
||||
@@ -1365,7 +1369,7 @@ impl AnalyticsReducer {
|
||||
let skill_id = skill_id_for_local_skill(
|
||||
repo_url.as_ref().map(SanitizedGitUrl::as_str),
|
||||
repo_root.as_deref(),
|
||||
path.as_path(),
|
||||
&path,
|
||||
invocation.skill_name.as_str(),
|
||||
);
|
||||
(skill_id, Some(skill_scope.to_string()))
|
||||
@@ -3835,7 +3839,7 @@ fn rejection_reason_from_error_type(
|
||||
pub(crate) fn skill_id_for_local_skill(
|
||||
repo_url: Option<&str>,
|
||||
repo_root: Option<&Path>,
|
||||
skill_path: &Path,
|
||||
skill_path: &PathUri,
|
||||
skill_name: &str,
|
||||
) -> String {
|
||||
let path = normalize_path_for_skill_id(repo_url, repo_root, skill_path);
|
||||
@@ -3857,10 +3861,14 @@ pub(crate) fn skill_id_for_local_skill(
|
||||
pub(crate) fn normalize_path_for_skill_id(
|
||||
repo_url: Option<&str>,
|
||||
repo_root: Option<&Path>,
|
||||
skill_path: &Path,
|
||||
skill_path: &PathUri,
|
||||
) -> String {
|
||||
// Foreign paths have no host repository to inspect; retain their native spelling for telemetry.
|
||||
let Ok(skill_path) = skill_path.to_abs_path() else {
|
||||
return skill_path.inferred_native_path_string().replace('\\', "/");
|
||||
};
|
||||
let resolved_path =
|
||||
std::fs::canonicalize(skill_path).unwrap_or_else(|_| skill_path.to_path_buf());
|
||||
std::fs::canonicalize(skill_path.as_path()).unwrap_or_else(|_| skill_path.to_path_buf());
|
||||
match (repo_url, repo_root) {
|
||||
(Some(_), Some(root)) => {
|
||||
let resolved_root = std::fs::canonicalize(root).unwrap_or_else(|_| root.to_path_buf());
|
||||
|
||||
@@ -11,6 +11,7 @@ use crate::reducer::normalize_path_for_skill_id;
|
||||
use crate::reducer::skill_id_for_local_skill;
|
||||
use crate::tests::support::TEST_PRODUCT_CLIENT_ID;
|
||||
use crate::tests::support::test_tracking_context;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::path::PathBuf;
|
||||
@@ -24,13 +25,13 @@ fn expected_absolute_path(path: &PathBuf) -> String {
|
||||
|
||||
#[test]
|
||||
fn normalize_path_for_skill_id_repo_scoped_uses_relative_path() {
|
||||
let repo_root = PathBuf::from("/repo/root");
|
||||
let skill_path = PathBuf::from("/repo/root/.codex/skills/doc/SKILL.md");
|
||||
let repo_root = std::env::temp_dir().join("repo/root");
|
||||
let skill_path = repo_root.join(".codex/skills/doc/SKILL.md");
|
||||
|
||||
let path = normalize_path_for_skill_id(
|
||||
Some("https://example.com/repo.git"),
|
||||
Some(repo_root.as_path()),
|
||||
skill_path.as_path(),
|
||||
&PathUri::from_host_native_path(&skill_path).expect("native skill path"),
|
||||
);
|
||||
|
||||
assert_eq!(path, ".codex/skills/doc/SKILL.md");
|
||||
@@ -38,12 +39,12 @@ fn normalize_path_for_skill_id_repo_scoped_uses_relative_path() {
|
||||
|
||||
#[test]
|
||||
fn normalize_path_for_skill_id_user_scoped_uses_absolute_path() {
|
||||
let skill_path = PathBuf::from("/Users/abc/.codex/skills/doc/SKILL.md");
|
||||
let skill_path = std::env::temp_dir().join("Users/abc/.codex/skills/doc/SKILL.md");
|
||||
|
||||
let path = normalize_path_for_skill_id(
|
||||
/*repo_url*/ None,
|
||||
/*repo_root*/ None,
|
||||
skill_path.as_path(),
|
||||
&PathUri::from_host_native_path(&skill_path).expect("native skill path"),
|
||||
);
|
||||
let expected = expected_absolute_path(&skill_path);
|
||||
|
||||
@@ -52,12 +53,12 @@ fn normalize_path_for_skill_id_user_scoped_uses_absolute_path() {
|
||||
|
||||
#[test]
|
||||
fn normalize_path_for_skill_id_admin_scoped_uses_absolute_path() {
|
||||
let skill_path = PathBuf::from("/etc/codex/skills/doc/SKILL.md");
|
||||
let skill_path = std::env::temp_dir().join("etc/codex/skills/doc/SKILL.md");
|
||||
|
||||
let path = normalize_path_for_skill_id(
|
||||
/*repo_url*/ None,
|
||||
/*repo_root*/ None,
|
||||
skill_path.as_path(),
|
||||
&PathUri::from_host_native_path(&skill_path).expect("native skill path"),
|
||||
);
|
||||
let expected = expected_absolute_path(&skill_path);
|
||||
|
||||
@@ -66,29 +67,56 @@ fn normalize_path_for_skill_id_admin_scoped_uses_absolute_path() {
|
||||
|
||||
#[test]
|
||||
fn normalize_path_for_skill_id_repo_root_not_in_skill_path_uses_absolute_path() {
|
||||
let repo_root = PathBuf::from("/repo/root");
|
||||
let skill_path = PathBuf::from("/other/path/.codex/skills/doc/SKILL.md");
|
||||
let repo_root = std::env::temp_dir().join("repo/root");
|
||||
let skill_path = std::env::temp_dir().join("other/path/.codex/skills/doc/SKILL.md");
|
||||
|
||||
let path = normalize_path_for_skill_id(
|
||||
Some("https://example.com/repo.git"),
|
||||
Some(repo_root.as_path()),
|
||||
skill_path.as_path(),
|
||||
&PathUri::from_host_native_path(&skill_path).expect("native skill path"),
|
||||
);
|
||||
let expected = expected_absolute_path(&skill_path);
|
||||
|
||||
assert_eq!(path, expected);
|
||||
}
|
||||
|
||||
/// Foreign paths keep native telemetry spelling without depending on host filesystem lookup.
|
||||
#[test]
|
||||
fn normalize_path_for_skill_id_foreign_path_uses_native_spelling() {
|
||||
let (uri, expected) = if cfg!(windows) {
|
||||
(
|
||||
"file:///Users/abc/.codex/skills/doc%20%23/SKILL.md",
|
||||
"/Users/abc/.codex/skills/doc #/SKILL.md",
|
||||
)
|
||||
} else {
|
||||
(
|
||||
"file:///C:/Users/abc/.codex/skills/doc%20%23/SKILL.md",
|
||||
"C:/Users/abc/.codex/skills/doc #/SKILL.md",
|
||||
)
|
||||
};
|
||||
let path = normalize_path_for_skill_id(
|
||||
/*repo_url*/ None,
|
||||
/*repo_root*/ None,
|
||||
&PathUri::parse(uri).expect("foreign skill URI"),
|
||||
);
|
||||
|
||||
assert_eq!(path, expected);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reducer_ingests_skill_invoked_fact() {
|
||||
let mut reducer = AnalyticsReducer::default();
|
||||
let mut events = Vec::new();
|
||||
let tracking = test_tracking_context("thread-1", "turn-1");
|
||||
let skill_path = PathBuf::from("/Users/abc/.codex/skills/doc/SKILL.md");
|
||||
let skill_path = std::env::temp_dir()
|
||||
.ancestors()
|
||||
.last()
|
||||
.expect("temporary directory root")
|
||||
.join("Users/abc/.codex/skills/doc/SKILL.md");
|
||||
let expected_skill_id = skill_id_for_local_skill(
|
||||
/*repo_url*/ None,
|
||||
/*repo_root*/ None,
|
||||
skill_path.as_path(),
|
||||
&PathUri::from_host_native_path(&skill_path).expect("native skill path"),
|
||||
"doc",
|
||||
);
|
||||
|
||||
@@ -99,7 +127,8 @@ async fn reducer_ingests_skill_invoked_fact() {
|
||||
invocations: vec![SkillInvocation {
|
||||
skill_name: "doc".to_string(),
|
||||
location: SkillInvocationLocation::Host {
|
||||
path: skill_path,
|
||||
path: PathUri::from_host_native_path(&skill_path)
|
||||
.expect("native skill path"),
|
||||
scope: codex_protocol::protocol::SkillScope::User,
|
||||
},
|
||||
plugin_id: None,
|
||||
@@ -138,8 +167,11 @@ async fn reducer_includes_plugin_ids_for_plugin_skill_invocations() {
|
||||
let mut reducer = AnalyticsReducer::default();
|
||||
let mut events = Vec::new();
|
||||
let tracking = test_tracking_context("thread-1", "turn-1");
|
||||
let skill_path =
|
||||
PathBuf::from("/Users/abc/.codex/plugins/cache/test/sample/skills/doc/SKILL.md");
|
||||
let skill_path = std::env::temp_dir()
|
||||
.ancestors()
|
||||
.last()
|
||||
.expect("temporary directory root")
|
||||
.join("Users/abc/.codex/plugins/cache/test/sample/skills/doc/SKILL.md");
|
||||
|
||||
reducer
|
||||
.ingest(
|
||||
@@ -148,7 +180,8 @@ async fn reducer_includes_plugin_ids_for_plugin_skill_invocations() {
|
||||
invocations: vec![SkillInvocation {
|
||||
skill_name: "sample:doc".to_string(),
|
||||
location: SkillInvocationLocation::Host {
|
||||
path: skill_path,
|
||||
path: PathUri::from_host_native_path(&skill_path)
|
||||
.expect("native skill path"),
|
||||
scope: codex_protocol::protocol::SkillScope::User,
|
||||
},
|
||||
plugin_id: Some("sample@test".to_string()),
|
||||
|
||||
+2
-2
@@ -20387,7 +20387,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/v2/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/v2/LegacyAppPathString"
|
||||
},
|
||||
"pluginId": {
|
||||
"description": "Owning plugin ID, matching `PluginSummary.id`, when known.",
|
||||
@@ -20460,7 +20460,7 @@
|
||||
"path": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/v2/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/v2/LegacyAppPathString"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
|
||||
+2
-2
@@ -18101,7 +18101,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"pluginId": {
|
||||
"description": "Owning plugin ID, matching `PluginSummary.id`, when known.",
|
||||
@@ -18174,7 +18174,7 @@
|
||||
"path": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
|
||||
@@ -125,6 +125,9 @@
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
"LegacyAppPathString": {
|
||||
"type": "string"
|
||||
},
|
||||
"PluginAuthPolicy": {
|
||||
"enum": [
|
||||
"ON_INSTALL",
|
||||
@@ -1020,7 +1023,7 @@
|
||||
"path": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
|
||||
@@ -5,6 +5,9 @@
|
||||
"description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.",
|
||||
"type": "string"
|
||||
},
|
||||
"LegacyAppPathString": {
|
||||
"type": "string"
|
||||
},
|
||||
"SkillDependencies": {
|
||||
"properties": {
|
||||
"tools": {
|
||||
@@ -129,7 +132,7 @@
|
||||
"type": "string"
|
||||
},
|
||||
"path": {
|
||||
"$ref": "#/definitions/AbsolutePathBuf"
|
||||
"$ref": "#/definitions/LegacyAppPathString"
|
||||
},
|
||||
"pluginId": {
|
||||
"description": "Owning plugin ID, matching `PluginSummary.id`, when known.",
|
||||
|
||||
BIN
Binary file not shown.
BIN
Binary file not shown.
@@ -1,7 +1,7 @@
|
||||
// GENERATED CODE! DO NOT MODIFY BY HAND!
|
||||
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
|
||||
import type { LegacyAppPathString } from "../LegacyAppPathString";
|
||||
import type { SkillDependencies } from "./SkillDependencies";
|
||||
import type { SkillInterface } from "./SkillInterface";
|
||||
import type { SkillScope } from "./SkillScope";
|
||||
@@ -10,7 +10,7 @@ export type SkillMetadata = { name: string, description: string,
|
||||
/**
|
||||
* Legacy short_description from SKILL.md. Prefer SKILL.json interface.short_description.
|
||||
*/
|
||||
shortDescription?: string, interface?: SkillInterface, dependencies?: SkillDependencies, path: AbsolutePathBuf, scope: SkillScope, enabled: boolean,
|
||||
shortDescription?: string, interface?: SkillInterface, dependencies?: SkillDependencies, path: LegacyAppPathString, scope: SkillScope, enabled: boolean,
|
||||
/**
|
||||
* Owning plugin ID, matching `PluginSummary.id`, when known.
|
||||
*/
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// GENERATED CODE! DO NOT MODIFY BY HAND!
|
||||
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
|
||||
import type { LegacyAppPathString } from "../LegacyAppPathString";
|
||||
import type { SkillInterface } from "./SkillInterface";
|
||||
|
||||
export type SkillSummary = { name: string, description: string, shortDescription: string | null, interface: SkillInterface | null, path: AbsolutePathBuf | null, enabled: boolean, };
|
||||
export type SkillSummary = { name: string, description: string, shortDescription: string | null, interface: SkillInterface | null, path: LegacyAppPathString | null, enabled: boolean, };
|
||||
|
||||
@@ -10,6 +10,7 @@ use codex_protocol::protocol::SkillMetadata as CoreSkillMetadata;
|
||||
use codex_protocol::protocol::SkillScope as CoreSkillScope;
|
||||
use codex_protocol::protocol::SkillToolDependency as CoreSkillToolDependency;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::path::PathBuf;
|
||||
@@ -477,7 +478,7 @@ pub struct SkillMetadata {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub dependencies: Option<SkillDependencies>,
|
||||
pub path: AbsolutePathBuf,
|
||||
pub path: LegacyAppPathString,
|
||||
pub scope: SkillScope,
|
||||
pub enabled: bool,
|
||||
/// Owning plugin ID, matching `PluginSummary.id`, when known.
|
||||
@@ -858,7 +859,7 @@ pub struct SkillSummary {
|
||||
pub description: String,
|
||||
pub short_description: Option<String>,
|
||||
pub interface: Option<SkillInterface>,
|
||||
pub path: Option<AbsolutePathBuf>,
|
||||
pub path: Option<LegacyAppPathString>,
|
||||
pub enabled: bool,
|
||||
}
|
||||
|
||||
@@ -989,7 +990,7 @@ impl From<CoreSkillMetadata> for SkillMetadata {
|
||||
short_description: value.short_description,
|
||||
interface: value.interface.map(SkillInterface::from),
|
||||
dependencies: value.dependencies.map(SkillDependencies::from),
|
||||
path: value.path,
|
||||
path: value.path.into(),
|
||||
scope: value.scope.into(),
|
||||
enabled: true,
|
||||
plugin_id: None,
|
||||
|
||||
@@ -22,7 +22,7 @@ const SKILLS_LIST_CWD_CONCURRENCY: usize = 5;
|
||||
|
||||
fn skills_to_info(
|
||||
skills: &[codex_skills::SkillMetadata],
|
||||
disabled_paths: &HashSet<AbsolutePathBuf>,
|
||||
disabled_paths: &HashSet<PathUri>,
|
||||
) -> Vec<codex_app_server_protocol::SkillMetadata> {
|
||||
skills
|
||||
.iter()
|
||||
@@ -60,7 +60,7 @@ fn skills_to_info(
|
||||
.collect(),
|
||||
}
|
||||
}),
|
||||
path: skill.path_to_skills_md.clone(),
|
||||
path: skill.path_to_skills_md.clone().into(),
|
||||
scope: skill.scope.into(),
|
||||
enabled,
|
||||
plugin_id: skill.plugin_id.clone(),
|
||||
|
||||
@@ -41,6 +41,8 @@ use codex_rmcp_client::McpOAuthClientRegistration;
|
||||
use codex_rmcp_client::OAuthDiscoveryTimeout;
|
||||
use codex_rmcp_client::StreamableHttpRedirectMode;
|
||||
use codex_rmcp_client::perform_oauth_login_silent;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
mod local;
|
||||
mod reconcile;
|
||||
@@ -67,7 +69,7 @@ pub(crate) struct PluginRequestProcessor {
|
||||
|
||||
fn plugin_skills_to_info<'a>(
|
||||
skills: impl IntoIterator<Item = &'a codex_skills::SkillMetadata>,
|
||||
disabled_skill_paths: &HashSet<AbsolutePathBuf>,
|
||||
disabled_skill_paths: &HashSet<PathUri>,
|
||||
) -> Vec<SkillSummary> {
|
||||
skills
|
||||
.into_iter()
|
||||
@@ -87,7 +89,7 @@ fn plugin_skills_to_info<'a>(
|
||||
default_prompt: interface.default_prompt,
|
||||
}
|
||||
}),
|
||||
path: Some(skill.path_to_skills_md.clone()),
|
||||
path: Some(skill.path_to_skills_md.clone().into()),
|
||||
enabled: !disabled_skill_paths.contains(&skill.path_to_skills_md),
|
||||
})
|
||||
.collect()
|
||||
@@ -1111,9 +1113,18 @@ impl PluginRequestProcessor {
|
||||
let onboarding_skill = if outcome.plugin.enabled
|
||||
&& let Some(path) = outcome.plugin.onboarding_skill.as_ref()
|
||||
{
|
||||
let path = PathUri::from_abs_path(path);
|
||||
skills
|
||||
.iter()
|
||||
.find(|skill| skill.enabled && skill.path.as_ref() == Some(path))
|
||||
.find(|skill| {
|
||||
skill.enabled
|
||||
&& skill
|
||||
.path
|
||||
.as_ref()
|
||||
.and_then(LegacyAppPathString::to_inferred_path_uri)
|
||||
.as_ref()
|
||||
== Some(&path)
|
||||
})
|
||||
.cloned()
|
||||
} else {
|
||||
None
|
||||
|
||||
@@ -133,7 +133,7 @@ async fn signed_image_renewal_preserves_live_mcp_and_plugin_skills() -> Result<(
|
||||
);
|
||||
assert!(warm_skills[0].enabled);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(warm_skills[0].path.as_path())?,
|
||||
std::fs::read_to_string(warm_skills[0].path.as_str())?,
|
||||
SKILL_CONTENTS
|
||||
);
|
||||
|
||||
@@ -169,7 +169,7 @@ async fn signed_image_renewal_preserves_live_mcp_and_plugin_skills() -> Result<(
|
||||
assert_eq!(sessions.load(Ordering::SeqCst), warm_sessions);
|
||||
assert_eq!(calls.snapshot().main_prompt_reads, 4);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(warm_skills[0].path.as_path())?,
|
||||
std::fs::read_to_string(warm_skills[0].path.as_str())?,
|
||||
SKILL_CONTENTS
|
||||
);
|
||||
server_handle.abort();
|
||||
|
||||
@@ -138,9 +138,12 @@ enabled = {skill_enabled}
|
||||
description: "Run setup".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: Some(AbsolutePathBuf::try_from(std::fs::canonicalize(
|
||||
plugin_root.join("skills/setup/SKILL.md"),
|
||||
)?)?),
|
||||
path: Some(
|
||||
AbsolutePathBuf::try_from(std::fs::canonicalize(
|
||||
plugin_root.join("skills/setup/SKILL.md"),
|
||||
)?)?
|
||||
.into(),
|
||||
),
|
||||
enabled: skill_enabled,
|
||||
};
|
||||
assert_eq!(response.plugin.summary.enabled, plugin_enabled);
|
||||
@@ -1606,9 +1609,9 @@ async fn plugin_read_agent_plugin_excludes_nested_skills() -> Result<()> {
|
||||
description: "Direct skill".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: Some(AbsolutePathBuf::try_from(std::fs::canonicalize(
|
||||
direct_skill_path
|
||||
)?)?),
|
||||
path: Some(
|
||||
AbsolutePathBuf::try_from(std::fs::canonicalize(direct_skill_path)?)?.into()
|
||||
),
|
||||
enabled: true,
|
||||
}]
|
||||
);
|
||||
|
||||
@@ -193,7 +193,7 @@ async fn skills_list_disabled_bundled_skills_preserves_shared_system_skill_cache
|
||||
assert!(
|
||||
system_skill_paths
|
||||
.iter()
|
||||
.all(|path| path.as_path().is_file()),
|
||||
.all(|path| std::path::Path::new(path.as_str()).is_file()),
|
||||
"disabled app-server must not remove the cache shared by other processes"
|
||||
);
|
||||
|
||||
@@ -659,7 +659,7 @@ async fn skills_list_loads_remote_installed_plugin_skills_from_cache() -> Result
|
||||
.find(|skill| skill.name == "linear:triage-issues")
|
||||
.expect("expected skill from cached remote plugin");
|
||||
assert_eq!(
|
||||
std::fs::canonicalize(skill.path.as_path())?,
|
||||
std::fs::canonicalize(skill.path.as_str())?,
|
||||
expected_skill_path
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -1003,14 +1003,14 @@ enabled = true
|
||||
Ok((
|
||||
skill.path.clone(),
|
||||
skill.description.clone(),
|
||||
std::fs::read_to_string(&skill.path)?,
|
||||
std::fs::read_to_string(skill.path.as_str())?,
|
||||
))
|
||||
})
|
||||
.collect::<std::io::Result<Vec<_>>>()?;
|
||||
assert_eq!(
|
||||
skills,
|
||||
vec![(
|
||||
expected_path.clone(),
|
||||
expected_path.clone().into(),
|
||||
format!("version {version}"),
|
||||
body.clone()
|
||||
)],
|
||||
|
||||
@@ -6,6 +6,7 @@ use std::num::NonZeroUsize;
|
||||
use crate::ConfigLayerSource;
|
||||
use crate::ConfigLayerStack;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
@@ -76,10 +77,10 @@ impl TryFrom<toml::Value> for SkillsConfig {
|
||||
}
|
||||
|
||||
/// Selects configured skills by their name or canonical document path.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
pub enum SkillConfigRuleSelector {
|
||||
Name(String),
|
||||
Path(AbsolutePathBuf),
|
||||
Path(PathUri),
|
||||
}
|
||||
|
||||
/// Enables or disables every skill matched by its selector.
|
||||
@@ -101,8 +102,8 @@ impl SkillConfigRules {
|
||||
/// Explicit path selectors remain effective even when no current skill matches.
|
||||
pub fn resolve_disabled_paths<'a>(
|
||||
&self,
|
||||
skills: impl IntoIterator<Item = (&'a str, &'a AbsolutePathBuf)> + Clone,
|
||||
) -> HashSet<AbsolutePathBuf> {
|
||||
skills: impl IntoIterator<Item = (&'a str, &'a PathUri)> + Clone,
|
||||
) -> HashSet<PathUri> {
|
||||
let mut disabled_paths = HashSet::new();
|
||||
|
||||
for entry in &self.entries {
|
||||
@@ -195,9 +196,9 @@ pub fn skill_config_rules_from_stack(config_layer_stack: &ConfigLayerStack) -> S
|
||||
|
||||
fn skill_config_rule_selector(entry: &SkillConfig) -> Option<SkillConfigRuleSelector> {
|
||||
match (entry.path.as_ref(), entry.name.as_deref()) {
|
||||
(Some(path), None) => Some(SkillConfigRuleSelector::Path(
|
||||
path.canonicalize().unwrap_or_else(|_| path.clone()),
|
||||
)),
|
||||
(Some(path), None) => Some(SkillConfigRuleSelector::Path(PathUri::from_abs_path(
|
||||
&path.canonicalize().unwrap_or_else(|_| path.clone()),
|
||||
))),
|
||||
(None, Some(name)) => {
|
||||
let name = name.trim();
|
||||
if name.is_empty() {
|
||||
|
||||
@@ -5,6 +5,7 @@ use crate::ConfigLayerStack;
|
||||
use crate::ConfigRequirementsToml;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
@@ -96,7 +97,7 @@ fn session_flags_can_reenable_user_disabled_path() {
|
||||
)),
|
||||
SkillConfigRules {
|
||||
entries: vec![SkillConfigRule {
|
||||
selector: SkillConfigRuleSelector::Path(skill_path.abs()),
|
||||
selector: SkillConfigRuleSelector::Path(PathUri::from_abs_path(&skill_path.abs())),
|
||||
enabled: true,
|
||||
}],
|
||||
}
|
||||
@@ -116,7 +117,7 @@ fn session_flags_can_disable_user_enabled_path() {
|
||||
)),
|
||||
SkillConfigRules {
|
||||
entries: vec![SkillConfigRule {
|
||||
selector: SkillConfigRuleSelector::Path(skill_path.abs()),
|
||||
selector: SkillConfigRuleSelector::Path(PathUri::from_abs_path(&skill_path.abs())),
|
||||
enabled: false,
|
||||
}],
|
||||
}
|
||||
@@ -164,7 +165,9 @@ enabled = true
|
||||
SkillConfigRules {
|
||||
entries: vec![
|
||||
SkillConfigRule {
|
||||
selector: SkillConfigRuleSelector::Path(skill_path.abs()),
|
||||
selector: SkillConfigRuleSelector::Path(PathUri::from_abs_path(
|
||||
&skill_path.abs()
|
||||
)),
|
||||
enabled: false,
|
||||
},
|
||||
SkillConfigRule {
|
||||
@@ -182,14 +185,14 @@ fn path_rule_disables_selected_path() {
|
||||
let path = codex_home.path().join("disable-by-path/SKILL.md").abs();
|
||||
let rules = SkillConfigRules {
|
||||
entries: vec![SkillConfigRule {
|
||||
selector: SkillConfigRuleSelector::Path(path.clone()),
|
||||
selector: SkillConfigRuleSelector::Path(PathUri::from_abs_path(&path)),
|
||||
enabled: false,
|
||||
}],
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
rules.resolve_disabled_paths(std::iter::empty()),
|
||||
[path].into_iter().collect()
|
||||
[PathUri::from_abs_path(&path)].into_iter().collect()
|
||||
);
|
||||
}
|
||||
|
||||
@@ -200,7 +203,7 @@ fn later_name_rule_reenables_path_disabled_skill() {
|
||||
let rules = SkillConfigRules {
|
||||
entries: vec![
|
||||
SkillConfigRule {
|
||||
selector: SkillConfigRuleSelector::Path(path.clone()),
|
||||
selector: SkillConfigRuleSelector::Path(PathUri::from_abs_path(&path)),
|
||||
enabled: false,
|
||||
},
|
||||
SkillConfigRule {
|
||||
@@ -211,7 +214,7 @@ fn later_name_rule_reenables_path_disabled_skill() {
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
rules.resolve_disabled_paths([("demo", &path)]),
|
||||
rules.resolve_disabled_paths([("demo", &PathUri::from_abs_path(&path))]),
|
||||
Default::default()
|
||||
);
|
||||
}
|
||||
@@ -229,14 +232,17 @@ fn later_path_rule_reenables_one_skill_disabled_by_name() {
|
||||
enabled: false,
|
||||
},
|
||||
SkillConfigRule {
|
||||
selector: SkillConfigRuleSelector::Path(first_path.clone()),
|
||||
selector: SkillConfigRuleSelector::Path(PathUri::from_abs_path(&first_path)),
|
||||
enabled: true,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
rules.resolve_disabled_paths([("demo", &first_path), ("demo", &second_path)]),
|
||||
[second_path].into_iter().collect()
|
||||
rules.resolve_disabled_paths([
|
||||
("demo", &PathUri::from_abs_path(&first_path)),
|
||||
("demo", &PathUri::from_abs_path(&second_path)),
|
||||
]),
|
||||
[PathUri::from_abs_path(&second_path)].into_iter().collect()
|
||||
);
|
||||
}
|
||||
|
||||
@@ -48,6 +48,7 @@ use codex_skills::SkillRootLoadRequest;
|
||||
use codex_skills::SkillRootLoader;
|
||||
use codex_skills::SkillRootSnapshots;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
@@ -1022,7 +1023,7 @@ impl PluginSkillInventory {
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ResolvedPluginSkills {
|
||||
pub skills: Vec<SkillMetadata>,
|
||||
pub disabled_skill_paths: HashSet<AbsolutePathBuf>,
|
||||
pub disabled_skill_paths: HashSet<PathUri>,
|
||||
pub had_errors: bool,
|
||||
}
|
||||
|
||||
@@ -1034,7 +1035,7 @@ impl ResolvedPluginSkills {
|
||||
|
||||
fn contains_enabled_skill(
|
||||
skills: &[SkillMetadata],
|
||||
disabled_skill_paths: &HashSet<AbsolutePathBuf>,
|
||||
disabled_skill_paths: &HashSet<PathUri>,
|
||||
) -> bool {
|
||||
skills
|
||||
.iter()
|
||||
|
||||
@@ -122,6 +122,7 @@ use codex_tools::DiscoverablePluginInfo;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::filter_request_plugin_install_discoverable_tools_for_client;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use std::collections::BTreeSet;
|
||||
@@ -469,7 +470,7 @@ pub struct PluginDetail {
|
||||
pub installed: bool,
|
||||
pub enabled: bool,
|
||||
pub skills: Vec<SkillMetadata>,
|
||||
pub disabled_skill_paths: HashSet<AbsolutePathBuf>,
|
||||
pub disabled_skill_paths: HashSet<PathUri>,
|
||||
/// Packaged onboarding path; callers apply visibility and enablement.
|
||||
pub onboarding_skill: Option<AbsolutePathBuf>,
|
||||
pub hooks: Vec<PluginHookSummary>,
|
||||
@@ -2729,10 +2730,11 @@ impl PluginsManager {
|
||||
if !path.as_path().starts_with(plugin_root.as_path()) {
|
||||
return None;
|
||||
}
|
||||
let path_uri = PathUri::from_abs_path(&path);
|
||||
resolved_skills
|
||||
.skills
|
||||
.iter()
|
||||
.any(|skill| skill.path_to_skills_md == path)
|
||||
.any(|skill| skill.path_to_skills_md == path_uri)
|
||||
.then_some(path)
|
||||
});
|
||||
let plugin_data_root = self.store.plugin_data_root(&plugin_id);
|
||||
|
||||
@@ -1844,7 +1844,7 @@ enabled = true
|
||||
|
||||
assert_eq!(
|
||||
outcome.plugins()[0].disabled_skill_paths,
|
||||
HashSet::from([skill_path])
|
||||
HashSet::from([PathUri::from_abs_path(&skill_path)])
|
||||
);
|
||||
assert!(!outcome.plugins()[0].has_enabled_skills);
|
||||
assert!(outcome.capability_summaries().is_empty());
|
||||
@@ -2295,7 +2295,10 @@ async fn install_plugin_materializes_default_command_skills() {
|
||||
resolved
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| skill.path_to_skills_md.clone())
|
||||
.map(|skill| skill
|
||||
.path_to_skills_md
|
||||
.to_abs_path()
|
||||
.expect("host skill path"))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
AbsolutePathBuf::from_absolute_path_checked(
|
||||
@@ -2421,7 +2424,12 @@ async fn load_plugin_skills_dedupes_overlapping_manifest_roots() {
|
||||
let skill_paths = resolved
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| skill.path_to_skills_md.clone())
|
||||
.map(|skill| {
|
||||
skill
|
||||
.path_to_skills_md
|
||||
.to_abs_path()
|
||||
.expect("host skill path")
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let canonical_skill_path = |path| {
|
||||
AbsolutePathBuf::from_absolute_path_checked(
|
||||
|
||||
@@ -20,6 +20,7 @@ use codex_skills::LoadedSkills;
|
||||
use codex_skills::SkillMetadata;
|
||||
use codex_skills::SkillRootLoadRequest;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
@@ -117,7 +118,9 @@ async fn curated_git_requirements_control_plugin_skills() -> Result<()> {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: AbsolutePathBuf::try_from(fs::canonicalize(skill_path)?)?,
|
||||
path_to_skills_md: PathUri::from_abs_path(&AbsolutePathBuf::try_from(
|
||||
fs::canonicalize(skill_path)?,
|
||||
)?),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some(plugin_id),
|
||||
remote_plugin_id: None,
|
||||
|
||||
@@ -42,6 +42,7 @@ use codex_skills::SkillRootLoadRequest;
|
||||
use codex_skills::SkillRootLoader;
|
||||
use codex_skills::parse_skill_frontmatter_metadata;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
use codex_utils_plugins::migrated_command_skills_root;
|
||||
@@ -201,9 +202,17 @@ impl SkillRootLoader<PluginSkillRoot> for TestSkillRootLoader {
|
||||
outcome.errors.extend(snapshot.errors);
|
||||
}
|
||||
outcome.skills.sort_by(|left, right| {
|
||||
left.name
|
||||
.cmp(&right.name)
|
||||
.then_with(|| left.path_to_skills_md.cmp(&right.path_to_skills_md))
|
||||
left.name.cmp(&right.name).then_with(|| {
|
||||
left.path_to_skills_md
|
||||
.to_abs_path()
|
||||
.expect("host test skill path")
|
||||
.cmp(
|
||||
&right
|
||||
.path_to_skills_md
|
||||
.to_abs_path()
|
||||
.expect("host test skill path"),
|
||||
)
|
||||
})
|
||||
});
|
||||
outcome
|
||||
})
|
||||
@@ -261,7 +270,10 @@ fn load_test_skill_root(root: &PluginSkillRoot) -> LoadedSkillRoot {
|
||||
});
|
||||
match parsed {
|
||||
Ok(parsed) => {
|
||||
discovery_paths.insert(canonical_path.clone(), path);
|
||||
discovery_paths.insert(
|
||||
PathUri::from_abs_path(&canonical_path),
|
||||
PathUri::from_abs_path(&path),
|
||||
);
|
||||
skills.push(SkillMetadata {
|
||||
name: format!("{}:{}", root.plugin_namespace, parsed.name),
|
||||
description: parsed.description,
|
||||
@@ -269,7 +281,7 @@ fn load_test_skill_root(root: &PluginSkillRoot) -> LoadedSkillRoot {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: canonical_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&canonical_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some(root.plugin_identity.plugin_id.clone()),
|
||||
remote_plugin_id: root.plugin_identity.remote_plugin_id.clone(),
|
||||
|
||||
@@ -1156,7 +1156,7 @@ async fn build_skills_and_plugins(
|
||||
.zip(injected_host_skills.iter())
|
||||
.filter_map(|(item, skill)| {
|
||||
(!injected_host_skill_prompts
|
||||
.contains_path(&skill.path_to_skills_md.to_string_lossy()))
|
||||
.contains_path(&skill.path_to_skills_md.inferred_native_path_string()))
|
||||
.then_some(item)
|
||||
})
|
||||
.collect(),
|
||||
|
||||
@@ -81,7 +81,7 @@ pub(crate) async fn emit_explicit_skill_invocations(
|
||||
.extension_data
|
||||
.get::<InjectedHostSkillPrompts>();
|
||||
for skill in injected_skills {
|
||||
let skill_resource = skill.path_to_skills_md.to_string_lossy();
|
||||
let skill_resource = skill.path_to_skills_md.inferred_native_path_string();
|
||||
if injected_host_skill_prompts
|
||||
.as_ref()
|
||||
.is_some_and(|prompts| prompts.is_superseded_path(&skill_resource))
|
||||
@@ -116,7 +116,7 @@ pub(crate) async fn emit_explicit_skill_invocations(
|
||||
.map(|skill| SkillInvocation {
|
||||
skill_name: skill.name.clone(),
|
||||
location: SkillInvocationLocation::Host {
|
||||
path: skill.path_to_skills_md.to_path_buf(),
|
||||
path: skill.path_to_skills_md.clone(),
|
||||
scope: skill.scope,
|
||||
},
|
||||
plugin_id: skill.plugin_id.clone(),
|
||||
@@ -155,7 +155,7 @@ pub(crate) async fn maybe_emit_implicit_skill_invocation(
|
||||
SkillScope::System => "system",
|
||||
SkillScope::Admin => "admin",
|
||||
};
|
||||
let skill_path = path.to_string_lossy().into_owned();
|
||||
let skill_path = path.inferred_native_path_string();
|
||||
let seen_key = format!("{skill_scope}:{skill_path}:{skill_name}");
|
||||
(skill_path, seen_key)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
|
||||
use crate::HostSkillsSnapshot;
|
||||
|
||||
use crate::catalog::SkillCatalog;
|
||||
@@ -29,14 +31,17 @@ impl CharacterRoutingCardSkillSelector {
|
||||
entry.authority.kind == SkillSourceKind::Host && entry.is_model_visible()
|
||||
})
|
||||
.take(MAX_CANDIDATES)
|
||||
.map(|(id, entry)| (entry.main_prompt.as_str().replace('\\', "/"), id))
|
||||
.filter_map(|(id, entry)| {
|
||||
let path = LegacyAppPathString::from_string(entry.main_prompt.as_str())
|
||||
.to_inferred_path_uri()?;
|
||||
Some((path, id))
|
||||
})
|
||||
.collect::<HashMap<_, _>>();
|
||||
let routing_fields = host_snapshot
|
||||
.into_iter()
|
||||
.flat_map(|snapshot| snapshot.outcome().skills.iter())
|
||||
.filter_map(|skill| {
|
||||
let id = host_skill_ids
|
||||
.get(&skill.path_to_skills_md.to_string_lossy().replace('\\', "/"))?;
|
||||
let id = host_skill_ids.get(&skill.path_to_skills_md)?;
|
||||
let interface = skill.interface.as_ref()?;
|
||||
let mut field = String::new();
|
||||
for value in [
|
||||
|
||||
@@ -5,6 +5,7 @@ use codex_skills::SkillInterface;
|
||||
use codex_skills::SkillMetadata;
|
||||
use codex_skills::SkillToolDependency;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use crate::catalog::SkillAuthority;
|
||||
@@ -34,7 +35,7 @@ fn host_interface_metadata_makes_unlisted_workflow_discoverable() {
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
|
||||
@@ -530,8 +530,9 @@ where
|
||||
.iter()
|
||||
.filter(|host_skill| host_skill.name == entry.name)
|
||||
{
|
||||
injected_host_skill_prompts
|
||||
.insert_superseded_path(host_skill.path_to_skills_md.to_string_lossy());
|
||||
injected_host_skill_prompts.insert_superseded_path(
|
||||
host_skill.path_to_skills_md.inferred_native_path_string(),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,14 +16,14 @@ use codex_utils_path_uri::PathUri;
|
||||
pub struct SkillLoadOutcome {
|
||||
pub skills: Vec<SkillMetadata>,
|
||||
pub errors: Vec<SkillError>,
|
||||
pub disabled_paths: HashSet<AbsolutePathBuf>,
|
||||
pub disabled_paths: HashSet<PathUri>,
|
||||
pub(crate) skill_roots: Vec<AbsolutePathBuf>,
|
||||
pub(crate) skill_root_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||
pub(crate) skill_discovery_path_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||
pub(crate) agent_plugin_skill_paths: HashSet<AbsolutePathBuf>,
|
||||
pub(crate) skill_root_by_path: Arc<HashMap<PathUri, AbsolutePathBuf>>,
|
||||
pub(crate) skill_discovery_path_by_path: Arc<HashMap<PathUri, PathUri>>,
|
||||
pub(crate) agent_plugin_skill_paths: HashSet<PathUri>,
|
||||
pub(crate) file_systems_by_skill_path: SkillFileSystemsByPath,
|
||||
pub(crate) implicit_skills_by_scripts_dir: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
|
||||
pub(crate) implicit_skills_by_doc_path: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
|
||||
pub(crate) implicit_skills_by_scripts_dir: Arc<HashMap<PathUri, SkillMetadata>>,
|
||||
pub(crate) implicit_skills_by_doc_path: Arc<HashMap<PathUri, SkillMetadata>>,
|
||||
}
|
||||
|
||||
impl SkillLoadOutcome {
|
||||
@@ -32,10 +32,10 @@ impl SkillLoadOutcome {
|
||||
skills: Vec<SkillMetadata>,
|
||||
errors: Vec<SkillError>,
|
||||
skill_roots: Vec<AbsolutePathBuf>,
|
||||
skill_root_by_path: HashMap<AbsolutePathBuf, AbsolutePathBuf>,
|
||||
skill_discovery_path_by_path: HashMap<AbsolutePathBuf, AbsolutePathBuf>,
|
||||
agent_plugin_skill_paths: HashSet<AbsolutePathBuf>,
|
||||
file_systems_by_skill_path: HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>,
|
||||
skill_root_by_path: HashMap<PathUri, AbsolutePathBuf>,
|
||||
skill_discovery_path_by_path: HashMap<PathUri, PathUri>,
|
||||
agent_plugin_skill_paths: HashSet<PathUri>,
|
||||
file_systems_by_skill_path: HashMap<PathUri, Arc<dyn ExecutorFileSystem>>,
|
||||
) -> Self {
|
||||
Self {
|
||||
skills,
|
||||
@@ -59,7 +59,7 @@ impl SkillLoadOutcome {
|
||||
.map(|skill| (skill, self.is_skill_enabled(skill)))
|
||||
}
|
||||
|
||||
pub(crate) fn with_disabled_paths(mut self, disabled_paths: HashSet<AbsolutePathBuf>) -> Self {
|
||||
pub(crate) fn with_disabled_paths(mut self, disabled_paths: HashSet<PathUri>) -> Self {
|
||||
self.disabled_paths = disabled_paths;
|
||||
let mut by_scripts_dir = HashMap::new();
|
||||
let mut by_doc_path = HashMap::new();
|
||||
@@ -68,12 +68,16 @@ impl SkillLoadOutcome {
|
||||
.iter()
|
||||
.filter(|skill| self.is_skill_enabled(skill))
|
||||
{
|
||||
let skill_doc_path = canonicalize_if_exists(&skill.path_to_skills_md);
|
||||
by_doc_path.insert(skill_doc_path, skill.clone());
|
||||
// Implicit command detection probes the host filesystem only.
|
||||
let Ok(path) = skill.path_to_skills_md.to_abs_path() else {
|
||||
continue;
|
||||
};
|
||||
let skill_doc_path = canonicalize_if_exists(&path);
|
||||
by_doc_path.insert(PathUri::from_abs_path(&skill_doc_path), skill.clone());
|
||||
|
||||
if let Some(skill_dir) = skill.path_to_skills_md.parent() {
|
||||
if let Some(skill_dir) = path.parent() {
|
||||
let scripts_dir = canonicalize_if_exists(&skill_dir.join("scripts"));
|
||||
by_scripts_dir.insert(scripts_dir, skill.clone());
|
||||
by_scripts_dir.insert(PathUri::from_abs_path(&scripts_dir), skill.clone());
|
||||
}
|
||||
}
|
||||
self.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir);
|
||||
@@ -87,15 +91,12 @@ impl SkillLoadOutcome {
|
||||
}
|
||||
|
||||
/// Returns the discovery root that supplied a loaded skill path.
|
||||
pub(crate) fn skill_root_for_path(&self, path: &AbsolutePathBuf) -> Option<&AbsolutePathBuf> {
|
||||
pub(crate) fn skill_root_for_path(&self, path: &PathUri) -> Option<&AbsolutePathBuf> {
|
||||
self.skill_root_by_path.get(path)
|
||||
}
|
||||
|
||||
/// Returns the logical path used to discover a canonical skill path.
|
||||
pub(crate) fn skill_discovery_path_for_path(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
) -> Option<&AbsolutePathBuf> {
|
||||
pub(crate) fn skill_discovery_path_for_path(&self, path: &PathUri) -> Option<&PathUri> {
|
||||
self.skill_discovery_path_by_path.get(path)
|
||||
}
|
||||
|
||||
@@ -117,19 +118,24 @@ impl SkillLoadOutcome {
|
||||
let fs = self
|
||||
.file_system_for_skill(skill)
|
||||
.unwrap_or_else(|| Arc::clone(&LOCAL_FS));
|
||||
let path = PathUri::from_abs_path(&skill.path_to_skills_md);
|
||||
fs.read_file_text(&path, ReadFileOptions::default(), /*sandbox*/ None)
|
||||
.await
|
||||
fs.read_file_text(
|
||||
&skill.path_to_skills_md,
|
||||
ReadFileOptions::default(),
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
impl codex_skills::ImplicitSkillLookup for SkillLoadOutcome {
|
||||
fn implicit_skill_for_scripts_dir(&self, path: &AbsolutePathBuf) -> Option<&SkillMetadata> {
|
||||
self.implicit_skills_by_scripts_dir.get(path)
|
||||
self.implicit_skills_by_scripts_dir
|
||||
.get(&PathUri::from_abs_path(path))
|
||||
}
|
||||
|
||||
fn implicit_skill_for_doc_path(&self, path: &AbsolutePathBuf) -> Option<&SkillMetadata> {
|
||||
self.implicit_skills_by_doc_path.get(path)
|
||||
self.implicit_skills_by_doc_path
|
||||
.get(&PathUri::from_abs_path(path))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -138,11 +144,11 @@ impl codex_skills::ExplicitSkillLookup for SkillLoadOutcome {
|
||||
&self.skills
|
||||
}
|
||||
|
||||
fn disabled_paths(&self) -> &HashSet<AbsolutePathBuf> {
|
||||
fn disabled_paths(&self) -> &HashSet<PathUri> {
|
||||
&self.disabled_paths
|
||||
}
|
||||
|
||||
fn skill_discovery_path_for_path(&self, path: &AbsolutePathBuf) -> Option<&AbsolutePathBuf> {
|
||||
fn skill_discovery_path_for_path(&self, path: &PathUri) -> Option<&PathUri> {
|
||||
SkillLoadOutcome::skill_discovery_path_for_path(self, path)
|
||||
}
|
||||
|
||||
@@ -153,17 +159,17 @@ impl codex_skills::ExplicitSkillLookup for SkillLoadOutcome {
|
||||
|
||||
#[derive(Clone, Default)]
|
||||
pub(crate) struct SkillFileSystemsByPath {
|
||||
values: Arc<HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>>,
|
||||
values: Arc<HashMap<PathUri, Arc<dyn ExecutorFileSystem>>>,
|
||||
}
|
||||
|
||||
impl SkillFileSystemsByPath {
|
||||
pub(crate) fn new(values: HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>) -> Self {
|
||||
pub(crate) fn new(values: HashMap<PathUri, Arc<dyn ExecutorFileSystem>>) -> Self {
|
||||
Self {
|
||||
values: Arc::new(values),
|
||||
}
|
||||
}
|
||||
|
||||
fn get(&self, path: &AbsolutePathBuf) -> Option<Arc<dyn ExecutorFileSystem>> {
|
||||
fn get(&self, path: &PathUri) -> Option<Arc<dyn ExecutorFileSystem>> {
|
||||
self.values.get(path).map(Arc::clone)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,7 +89,7 @@ impl HostSkillsSnapshot {
|
||||
}
|
||||
prompts.fragments.push(Box::new(SkillInstructions {
|
||||
name: skill.name.clone(),
|
||||
path: skill.path_to_skills_md.to_string_lossy().into_owned(),
|
||||
path: skill.path_to_skills_md.inferred_native_path_string(),
|
||||
contents,
|
||||
resource_access: None,
|
||||
}));
|
||||
@@ -99,7 +99,7 @@ impl HostSkillsSnapshot {
|
||||
prompts.warnings.push(format!(
|
||||
"Failed to load skill {} at {}: {err:#}",
|
||||
skill.name,
|
||||
skill.path_to_skills_md.display()
|
||||
skill.path_to_skills_md.inferred_native_path_string()
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -234,7 +234,7 @@ fn expected_skill(path: AbsolutePathBuf, name: &str, scope: SkillScope) -> Skill
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&path),
|
||||
scope,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
|
||||
@@ -12,6 +12,7 @@ use codex_skills::SkillRootSnapshots;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_absolute_path::test_support::PathExt;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
@@ -409,9 +410,11 @@ async fn snapshot_for_config_preserves_host_precedence_for_symlinked_plugin_root
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: dunce::canonicalize(plugin_skill_path)
|
||||
.expect("canonical plugin skill path")
|
||||
.abs(),
|
||||
path_to_skills_md: PathUri::from_abs_path(
|
||||
&dunce::canonicalize(plugin_skill_path)
|
||||
.expect("canonical plugin skill path")
|
||||
.abs()
|
||||
),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -729,13 +732,9 @@ async fn skills_for_config_disables_plugin_skills_by_name() {
|
||||
.expect("skill path should canonicalize")
|
||||
.abs();
|
||||
|
||||
assert_eq!(skill.path_to_skills_md, skill_path);
|
||||
assert_eq!(skill.path_to_skills_md, PathUri::from_abs_path(&skill_path));
|
||||
assert!(outcome.disabled_paths.contains(&skill.path_to_skills_md));
|
||||
assert!(
|
||||
outcome
|
||||
.implicit_skill_for_doc_path(&skill.path_to_skills_md)
|
||||
.is_none()
|
||||
);
|
||||
assert!(outcome.implicit_skill_for_doc_path(&skill_path).is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -33,7 +33,7 @@ pub fn detect_implicit_skill_invocation(
|
||||
return Some(SkillInvocation {
|
||||
skill_name: skill.name,
|
||||
location: SkillInvocationLocation::Host {
|
||||
path: skill.path_to_skills_md.to_path_buf(),
|
||||
path: skill.path_to_skills_md,
|
||||
scope: skill.scope,
|
||||
},
|
||||
plugin_id: skill.plugin_id,
|
||||
|
||||
@@ -115,7 +115,7 @@ impl HostSkillRoot {
|
||||
pub(crate) struct HostSkillRootSnapshot {
|
||||
pub(crate) root: AbsolutePathBuf,
|
||||
pub(crate) skills: Vec<SkillMetadata>,
|
||||
pub(crate) skill_discovery_path_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||
pub(crate) skill_discovery_path_by_path: Arc<HashMap<PathUri, PathUri>>,
|
||||
pub(crate) errors: Vec<SkillError>,
|
||||
pub(crate) file_system: Arc<dyn ExecutorFileSystem>,
|
||||
pub(crate) is_agent_plugin: bool,
|
||||
@@ -148,7 +148,7 @@ async fn load_skills_under_root(
|
||||
root: &AbsolutePathBuf,
|
||||
) -> (
|
||||
Vec<SkillMetadata>,
|
||||
Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||
Arc<HashMap<PathUri, PathUri>>,
|
||||
Vec<SkillError>,
|
||||
) {
|
||||
let file_system = skill_root.file_system.as_ref();
|
||||
@@ -324,8 +324,10 @@ async fn load_skills_under_root(
|
||||
});
|
||||
match result {
|
||||
Ok(skill) => {
|
||||
skill_discovery_path_by_path
|
||||
.insert(skill.path_to_skills_md.clone(), discovery_path);
|
||||
skill_discovery_path_by_path.insert(
|
||||
skill.path_to_skills_md.clone(),
|
||||
PathUri::from_abs_path(&discovery_path),
|
||||
);
|
||||
loaded_skills.push(skill);
|
||||
}
|
||||
Err(message) if skill_root.scope != SkillScope::System => {
|
||||
@@ -384,7 +386,7 @@ async fn parse_skill_file(
|
||||
interface,
|
||||
dependencies,
|
||||
policy,
|
||||
path_to_skills_md: path.clone(),
|
||||
path_to_skills_md: path_uri.clone(),
|
||||
scope,
|
||||
plugin_id: plugin_identity.map(|identity| identity.plugin_id.clone()),
|
||||
remote_plugin_id: plugin_identity.and_then(|identity| identity.remote_plugin_id.clone()),
|
||||
|
||||
@@ -90,7 +90,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
|
||||
allow_implicit_invocation: Some(false),
|
||||
products: Vec::new(),
|
||||
}),
|
||||
path_to_skills_md: first_skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&first_skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -102,7 +102,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: second_skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&second_skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
|
||||
@@ -7,7 +7,6 @@ use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_skills::LoadedSkillRoot;
|
||||
use codex_skills::SkillRootSnapshots;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::migrated_command_skills_root;
|
||||
@@ -160,8 +159,7 @@ pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots(
|
||||
!migrated_command_root.as_ref().is_some_and(|migrated_root| {
|
||||
skill
|
||||
.path_to_skills_md
|
||||
.as_path()
|
||||
.starts_with(migrated_root.as_path())
|
||||
.starts_with(&PathUri::from_abs_path(migrated_root))
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -180,8 +178,7 @@ pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots(
|
||||
snapshot.skills.retain(|skill| {
|
||||
!skill
|
||||
.path_to_skills_md
|
||||
.as_path()
|
||||
.starts_with(migrated_command_root.as_path())
|
||||
.starts_with(&PathUri::from_abs_path(migrated_command_root))
|
||||
|| skill.plugin_id.as_ref().is_none_or(|plugin_id| {
|
||||
native_plugin_skill_names
|
||||
.get(plugin_id)
|
||||
@@ -205,8 +202,7 @@ fn merge_host_skill_root_snapshots(snapshots: Vec<HostSkillRootSnapshot>) -> Ski
|
||||
let mut skill_root_by_path = HashMap::new();
|
||||
let mut skill_discovery_path_by_path = HashMap::new();
|
||||
let mut agent_plugin_skill_paths = HashSet::new();
|
||||
let mut file_systems_by_skill_path =
|
||||
HashMap::<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>::new();
|
||||
let mut file_systems_by_skill_path = HashMap::<PathUri, Arc<dyn ExecutorFileSystem>>::new();
|
||||
|
||||
for snapshot in snapshots {
|
||||
if !snapshot.skills.is_empty() && !skill_roots.contains(&snapshot.root) {
|
||||
@@ -241,11 +237,13 @@ fn merge_host_skill_root_snapshots(snapshots: Vec<HostSkillRootSnapshot>) -> Ski
|
||||
skill_roots.retain(|root| retained_roots.contains(root));
|
||||
file_systems_by_skill_path.retain(|path, _| retained_paths.contains(path));
|
||||
agent_plugin_skill_paths.retain(|path| retained_paths.contains(path));
|
||||
skills.sort_by(|left, right| {
|
||||
scope_rank(left.scope)
|
||||
.cmp(&scope_rank(right.scope))
|
||||
.then_with(|| left.name.cmp(&right.name))
|
||||
.then_with(|| left.path_to_skills_md.cmp(&right.path_to_skills_md))
|
||||
// Keep native path ordering: URI escaping can reorder same-name skills.
|
||||
skills.sort_by_cached_key(|skill| {
|
||||
(
|
||||
scope_rank(skill.scope),
|
||||
skill.name.clone(),
|
||||
skill.path_to_skills_md.to_abs_path().ok(),
|
||||
)
|
||||
});
|
||||
|
||||
SkillLoadOutcome::from_parts(
|
||||
|
||||
@@ -426,7 +426,9 @@ async fn deduplicated_symlinked_skill_preserves_first_discovery_path() {
|
||||
),
|
||||
(
|
||||
Some(&canonical_first_root),
|
||||
Some(&canonical_first_root.join("first-link/SKILL.md")),
|
||||
Some(&PathUri::from_abs_path(
|
||||
&canonical_first_root.join("first-link/SKILL.md")
|
||||
)),
|
||||
)
|
||||
);
|
||||
}
|
||||
@@ -506,3 +508,39 @@ async fn merges_host_root_results_in_input_order_when_scans_finish_out_of_order(
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// Equal-name skills retain native path order so URI escaping does not reorder the catalog.
|
||||
#[tokio::test]
|
||||
async fn same_name_skills_preserve_native_path_order() {
|
||||
let temp = TempDir::new().expect("temp dir");
|
||||
let root = dunce::canonicalize(temp.path())
|
||||
.expect("canonical skill root")
|
||||
.abs();
|
||||
write_skill(&root, "é", "demo", "Accented path");
|
||||
write_skill(&root, "a", "demo", "ASCII path");
|
||||
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
vec![HostSkillRoot::host(
|
||||
root.clone(),
|
||||
SkillScope::User,
|
||||
Arc::clone(&LOCAL_FS),
|
||||
)],
|
||||
&Semaphore::new(/*permits*/ 1),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
assert_eq!(
|
||||
outcome
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| &skill.path_to_skills_md)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
&PathUri::from_abs_path(&root.join("a/SKILL.md")),
|
||||
&PathUri::from_abs_path(&root.join("é/SKILL.md")),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ use codex_skills::SkillMetadata;
|
||||
use codex_skills::SkillPolicy;
|
||||
use codex_skills::SkillToolDependency;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
@@ -150,7 +151,7 @@ policy:
|
||||
allow_implicit_invocation: Some(false),
|
||||
products: vec![Product::Codex, Product::Chatgpt, Product::Atlas],
|
||||
}),
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -176,7 +177,7 @@ async fn invalid_optional_metadata_fails_open() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -222,7 +223,7 @@ async fn loads_host_interface_metadata_and_local_asset_paths() {
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -265,7 +266,7 @@ async fn loads_plugin_skill_interface_icons_from_local_and_shared_assets() {
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: fixture.skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&fixture.skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some("fixture@test".to_string()),
|
||||
remote_plugin_id: None,
|
||||
@@ -303,7 +304,7 @@ async fn rejects_plugin_skill_interface_icons_outside_shared_assets() {
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: fixture.skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&fixture.skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some("fixture@test".to_string()),
|
||||
remote_plugin_id: None,
|
||||
@@ -333,7 +334,7 @@ async fn rejects_interface_fields_that_escape_or_fail_validation() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -355,7 +356,10 @@ async fn skips_hidden_host_skills() {
|
||||
|
||||
assert_eq!(snapshot.errors, Vec::new());
|
||||
assert_eq!(snapshot.skills.len(), 1);
|
||||
assert_eq!(snapshot.skills[0].path_to_skills_md, visible_path);
|
||||
assert_eq!(
|
||||
snapshot.skills[0].path_to_skills_md,
|
||||
PathUri::from_abs_path(&visible_path)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -383,7 +387,7 @@ async fn discovers_nested_plugin_namespace_without_plugin_identity() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -428,7 +432,7 @@ async fn plugin_root_accepts_maximum_length_qualified_skill_name() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some("demo@test".to_string()),
|
||||
remote_plugin_id: None,
|
||||
@@ -529,15 +533,17 @@ async fn recursive_plugin_root_preserves_owner_namespace_and_shared_asset_policy
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path.clone(),
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some("demo@test".to_string()),
|
||||
remote_plugin_id: Some("remote-demo".to_string()),
|
||||
}]
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.skill_discovery_path_by_path.get(&skill_path),
|
||||
Some(&skill_path)
|
||||
snapshot
|
||||
.skill_discovery_path_by_path
|
||||
.get(&PathUri::from_abs_path(&skill_path)),
|
||||
Some(&PathUri::from_abs_path(&skill_path))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -581,7 +587,7 @@ async fn direct_child_plugin_root_ignores_nested_skills() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: direct_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&direct_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some("demo@test".to_string()),
|
||||
remote_plugin_id: None,
|
||||
@@ -662,15 +668,19 @@ async fn recursive_plugin_root_preserves_symlinked_skill_discovery_path() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: target_skill.clone(),
|
||||
path_to_skills_md: PathUri::from_abs_path(&target_skill),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: Some("demo@test".to_string()),
|
||||
remote_plugin_id: None,
|
||||
}]
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.skill_discovery_path_by_path.get(&target_skill),
|
||||
Some(&snapshot.root.join("alias/SKILL.md"))
|
||||
snapshot
|
||||
.skill_discovery_path_by_path
|
||||
.get(&PathUri::from_abs_path(&target_skill)),
|
||||
Some(&PathUri::from_abs_path(
|
||||
&snapshot.root.join("alias/SKILL.md")
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
@@ -694,11 +704,15 @@ async fn follows_directory_symlinks_except_for_system_scope() {
|
||||
&snapshot.skills[0].path_to_skills_md,
|
||||
snapshot.skills[0].scope
|
||||
),
|
||||
(&target_skill, scope)
|
||||
(&PathUri::from_abs_path(&target_skill), scope)
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.skill_discovery_path_by_path.get(&target_skill),
|
||||
Some(&snapshot.root.join("alias/SKILL.md"))
|
||||
snapshot
|
||||
.skill_discovery_path_by_path
|
||||
.get(&PathUri::from_abs_path(&target_skill)),
|
||||
Some(&PathUri::from_abs_path(
|
||||
&snapshot.root.join("alias/SKILL.md")
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::collections::HashMap;
|
||||
|
||||
use crate::SkillLoadOutcome;
|
||||
use codex_skills::SkillMetadata;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
use crate::catalog::SkillAuthority;
|
||||
use crate::catalog::SkillCatalog;
|
||||
@@ -60,11 +61,17 @@ impl SkillProvider for HostSkillProvider {
|
||||
"host skill provider requires a host skills snapshot",
|
||||
));
|
||||
};
|
||||
let Some(skill) = host_snapshot.outcome().skills.iter().find(|skill| {
|
||||
let skill_path = skill.path_to_skills_md.to_string_lossy();
|
||||
skill_path == request.resource.as_str()
|
||||
|| skill_path.replace('\\', "/") == request.resource.as_str()
|
||||
}) else {
|
||||
// TODO(anp): Infer resource path conventions once foreign repository paths can reach this
|
||||
// provider. Despite its name, it also serves repository skills from the caller's environment;
|
||||
// the loader currently rejects foreign paths. Host-owned skills use the app-server's local
|
||||
// filesystem.
|
||||
let resource_path = PathUri::from_host_native_path(request.resource.as_str()).ok();
|
||||
let Some(skill) = host_snapshot
|
||||
.outcome()
|
||||
.skills
|
||||
.iter()
|
||||
.find(|skill| Some(&skill.path_to_skills_md) == resource_path.as_ref())
|
||||
else {
|
||||
return Err(SkillProviderError::new(format!(
|
||||
"host skill resource is not loaded: {}",
|
||||
request.resource.as_str()
|
||||
@@ -116,9 +123,15 @@ fn catalog_from_outcome(outcome: &SkillLoadOutcome) -> SkillCatalog {
|
||||
if let Some(discovery_path) =
|
||||
outcome.skill_discovery_path_for_path(&skill.path_to_skills_md)
|
||||
{
|
||||
entry = entry.with_display_path(discovery_path.to_string_lossy().replace('\\', "/"));
|
||||
// Keep the catalog's existing forward-slash display on Windows; lookups use PathUri.
|
||||
entry = entry.with_display_path(
|
||||
discovery_path
|
||||
.inferred_native_path_string()
|
||||
.replace('\\', "/"),
|
||||
);
|
||||
}
|
||||
if let Some(root) = outcome.skill_root_for_path(&skill.path_to_skills_md) {
|
||||
// Alias labels share the display path's separators so catalog rendering stays stable.
|
||||
entry = entry.with_alias_root(root.to_string_lossy().replace('\\', "/"));
|
||||
if let Some(root_order) = root_order_by_path.get(root.as_path()) {
|
||||
entry = entry.with_alias_root_order(*root_order);
|
||||
@@ -131,7 +144,8 @@ fn catalog_from_outcome(outcome: &SkillLoadOutcome) -> SkillCatalog {
|
||||
}
|
||||
|
||||
fn catalog_entry_from_skill(skill: &SkillMetadata, enabled: bool) -> SkillCatalogEntry {
|
||||
let skill_path = skill.path_to_skills_md.to_string_lossy().into_owned();
|
||||
let skill_path = skill.path_to_skills_md.inferred_native_path_string();
|
||||
// Preserve model-visible paths while resource IDs retain the native spelling used by clients.
|
||||
let display_path = skill_path.replace('\\', "/");
|
||||
let mut entry = SkillCatalogEntry::new(
|
||||
SkillPackageId(skill_path.clone()),
|
||||
|
||||
@@ -8,9 +8,16 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use super::HostSkillProvider;
|
||||
use super::catalog_from_outcome;
|
||||
use crate::HostSkillsSnapshot;
|
||||
use crate::catalog::SkillReadResult;
|
||||
use crate::catalog::SkillResourceId;
|
||||
use crate::loader::HostSkillRoot;
|
||||
use crate::loader::load_and_merge_host_skill_roots;
|
||||
use crate::provider::SkillProvider;
|
||||
use crate::provider::SkillReadContext;
|
||||
use crate::provider::SkillReadRequest;
|
||||
|
||||
#[tokio::test]
|
||||
async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dyn std::error::Error>>
|
||||
@@ -61,6 +68,63 @@ async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dy
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Host catalog IDs and displayed paths must resolve to the same discovered file,
|
||||
/// including Windows slash spelling and characters that need URI escaping.
|
||||
#[tokio::test]
|
||||
async fn host_catalog_resource_spellings_read_the_discovered_skill()
|
||||
-> Result<(), Box<dyn std::error::Error>> {
|
||||
let root = tempfile::tempdir()?;
|
||||
let skill_path = root.path().join("demo skill%23").join("SKILL.md");
|
||||
std::fs::create_dir_all(
|
||||
skill_path
|
||||
.parent()
|
||||
.ok_or("skill path should have a parent")?,
|
||||
)?;
|
||||
let contents = "---\nname: demo\ndescription: Demo skill.\n---\n# Demo\n";
|
||||
std::fs::write(&skill_path, contents)?;
|
||||
let root_path = AbsolutePathBuf::try_from(std::fs::canonicalize(root.path())?)?;
|
||||
let outcome = load_and_merge_host_skill_roots(
|
||||
vec![HostSkillRoot::host(
|
||||
root_path,
|
||||
SkillScope::User,
|
||||
Arc::clone(&LOCAL_FS),
|
||||
)],
|
||||
&Semaphore::new(/*permits*/ 1),
|
||||
/*restriction_product*/ None,
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
)
|
||||
.await;
|
||||
let catalog = catalog_from_outcome(&outcome);
|
||||
let entry = catalog
|
||||
.entries
|
||||
.first()
|
||||
.ok_or("skill should be discovered")?;
|
||||
let snapshot = Arc::new(HostSkillsSnapshot::new(Arc::new(outcome)));
|
||||
for resource in [
|
||||
entry.main_prompt.clone(),
|
||||
SkillResourceId::new(entry.display_path.as_ref().ok_or("skill display path")?),
|
||||
] {
|
||||
let result = HostSkillProvider::new()
|
||||
.read(SkillReadRequest {
|
||||
authority: entry.authority.clone(),
|
||||
package: entry.id.clone(),
|
||||
resource: resource.clone(),
|
||||
context: SkillReadContext::Host {
|
||||
host_snapshot: Some(Arc::clone(&snapshot)),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(
|
||||
result,
|
||||
SkillReadResult {
|
||||
resource,
|
||||
contents: contents.to_string()
|
||||
}
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn host_catalog_preserves_symlinked_skill_discovery_paths()
|
||||
|
||||
@@ -304,7 +304,7 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() {
|
||||
assert_eq!(skill.name, "synthetic");
|
||||
assert_eq!(
|
||||
skill.path_to_skills_md,
|
||||
canonical_root.join("skill/SKILL.md")
|
||||
PathUri::from_abs_path(&canonical_root.join("skill/SKILL.md"))
|
||||
);
|
||||
assert_eq!(
|
||||
snapshot.read_skill_text(&skill).await.expect("skill body"),
|
||||
|
||||
@@ -230,7 +230,7 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -311,7 +311,7 @@ async fn host_world_state_records_catalog_metrics_on_publish_and_change() -> Tes
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -380,7 +380,7 @@ async fn host_world_state_records_catalog_metrics_on_publish_and_change() -> Tes
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: second_skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&second_skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -436,7 +436,7 @@ async fn persisted_host_snapshot_deduplicates_warning_after_reinitialization() -
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_path,
|
||||
path_to_skills_md: PathUri::from_abs_path(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginIdentity;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use codex_utils_plugins::SkillDiscoveryMode;
|
||||
@@ -26,7 +27,7 @@ pub struct LoadedPlugin<M> {
|
||||
pub enabled: bool,
|
||||
pub skill_roots: Vec<AbsolutePathBuf>,
|
||||
pub skill_discovery_mode: SkillDiscoveryMode,
|
||||
pub disabled_skill_paths: HashSet<AbsolutePathBuf>,
|
||||
pub disabled_skill_paths: HashSet<PathUri>,
|
||||
pub has_enabled_skills: bool,
|
||||
pub mcp_servers: HashMap<String, M>,
|
||||
pub apps: Vec<AppDeclaration>,
|
||||
|
||||
@@ -31,7 +31,7 @@ fn test_skill_metadata(skill_doc_path: AbsolutePathBuf) -> SkillMetadata {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: skill_doc_path,
|
||||
path_to_skills_md: skill_doc_path.into(),
|
||||
scope: codex_protocol::protocol::SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
|
||||
@@ -8,6 +8,7 @@ use std::sync::Arc;
|
||||
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
use crate::SkillMetadata;
|
||||
|
||||
@@ -23,7 +24,7 @@ pub struct SkillError {
|
||||
pub struct LoadedSkillRoot {
|
||||
pub root: AbsolutePathBuf,
|
||||
pub skills: Vec<SkillMetadata>,
|
||||
pub skill_discovery_path_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||
pub skill_discovery_path_by_path: Arc<HashMap<PathUri, PathUri>>,
|
||||
pub errors: Vec<SkillError>,
|
||||
pub is_agent_plugin: bool,
|
||||
}
|
||||
|
||||
@@ -13,7 +13,7 @@ pub struct SkillMetadata {
|
||||
pub dependencies: Option<SkillDependencies>,
|
||||
pub policy: Option<SkillPolicy>,
|
||||
/// Path to the SKILL.md file that declares this skill.
|
||||
pub path_to_skills_md: AbsolutePathBuf,
|
||||
pub path_to_skills_md: PathUri,
|
||||
pub scope: SkillScope,
|
||||
pub plugin_id: Option<String>,
|
||||
pub remote_plugin_id: Option<String>,
|
||||
|
||||
@@ -1,14 +1,14 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
use crate::SkillMetadata;
|
||||
|
||||
/// Counts how often each skill name appears (exact and ASCII-lowercase), excluding disabled paths.
|
||||
pub fn build_skill_name_counts(
|
||||
skills: &[SkillMetadata],
|
||||
disabled_paths: &HashSet<AbsolutePathBuf>,
|
||||
disabled_paths: &HashSet<PathUri>,
|
||||
) -> (HashMap<String, usize>, HashMap<String, usize>) {
|
||||
let mut exact_counts: HashMap<String, usize> = HashMap::new();
|
||||
let mut lower_counts: HashMap<String, usize> = HashMap::new();
|
||||
|
||||
@@ -3,6 +3,8 @@ use std::collections::HashSet;
|
||||
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
use crate::SkillMetadata;
|
||||
use crate::ToolMentionKind;
|
||||
@@ -19,9 +21,9 @@ use crate::tool_kind_for_path;
|
||||
pub trait ExplicitSkillLookup {
|
||||
fn skills(&self) -> &[SkillMetadata];
|
||||
|
||||
fn disabled_paths(&self) -> &HashSet<AbsolutePathBuf>;
|
||||
fn disabled_paths(&self) -> &HashSet<PathUri>;
|
||||
|
||||
fn skill_discovery_path_for_path(&self, path: &AbsolutePathBuf) -> Option<&AbsolutePathBuf>;
|
||||
fn skill_discovery_path_for_path(&self, path: &PathUri) -> Option<&PathUri>;
|
||||
|
||||
fn is_skill_enabled(&self, skill: &SkillMetadata) -> bool {
|
||||
!self.disabled_paths().contains(&skill.path_to_skills_md)
|
||||
@@ -54,7 +56,7 @@ pub fn collect_explicit_skill_mentions(
|
||||
};
|
||||
let mut selected: Vec<SkillMetadata> = Vec::new();
|
||||
let mut seen_names: HashSet<String> = HashSet::new();
|
||||
let mut seen_paths: HashSet<AbsolutePathBuf> = HashSet::new();
|
||||
let mut seen_paths: HashSet<PathUri> = HashSet::new();
|
||||
let mut blocked_plain_names: HashSet<String> = HashSet::new();
|
||||
|
||||
for input in inputs {
|
||||
@@ -63,6 +65,7 @@ pub fn collect_explicit_skill_mentions(
|
||||
let Ok(path) = AbsolutePathBuf::relative_to_current_dir(path) else {
|
||||
continue;
|
||||
};
|
||||
let path = PathUri::from_abs_path(&path);
|
||||
|
||||
let Some(skill) = selection_context
|
||||
.loaded_skills
|
||||
@@ -120,14 +123,14 @@ fn select_skills_from_mentions(
|
||||
blocked_plain_names: &HashSet<String>,
|
||||
mentions: &ToolMentions<'_>,
|
||||
seen_names: &mut HashSet<String>,
|
||||
seen_paths: &mut HashSet<AbsolutePathBuf>,
|
||||
seen_paths: &mut HashSet<PathUri>,
|
||||
selected: &mut Vec<SkillMetadata>,
|
||||
) {
|
||||
if mentions.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let mention_skill_paths: HashSet<String> = mentions
|
||||
let mention_skill_paths: HashSet<PathUri> = mentions
|
||||
.paths()
|
||||
.filter(|path| {
|
||||
!matches!(
|
||||
@@ -135,7 +138,12 @@ fn select_skills_from_mentions(
|
||||
ToolMentionKind::App | ToolMentionKind::Mcp | ToolMentionKind::Plugin
|
||||
)
|
||||
})
|
||||
.map(normalize_host_skill_path)
|
||||
.filter_map(|path| {
|
||||
let path = normalize_skill_path(path);
|
||||
PathUri::from_host_native_path(path)
|
||||
.ok()
|
||||
.or_else(|| LegacyAppPathString::from_string(path).to_inferred_path_uri())
|
||||
})
|
||||
.collect();
|
||||
|
||||
for skill in selection_context.loaded_skills.skills() {
|
||||
@@ -145,16 +153,11 @@ fn select_skills_from_mentions(
|
||||
continue;
|
||||
}
|
||||
|
||||
let canonical_path = normalize_host_skill_path(&skill.path_to_skills_md.to_string_lossy());
|
||||
let matches_discovery_path = selection_context
|
||||
.loaded_skills
|
||||
.skill_discovery_path_for_path(&skill.path_to_skills_md)
|
||||
.is_some_and(|discovery_path| {
|
||||
mention_skill_paths.contains(&normalize_host_skill_path(
|
||||
&discovery_path.to_string_lossy(),
|
||||
))
|
||||
});
|
||||
if mention_skill_paths.contains(&canonical_path) || matches_discovery_path {
|
||||
.is_some_and(|discovery_path| mention_skill_paths.contains(discovery_path));
|
||||
if mention_skill_paths.contains(&skill.path_to_skills_md) || matches_discovery_path {
|
||||
seen_paths.insert(skill.path_to_skills_md.clone());
|
||||
seen_names.insert(skill.name.clone());
|
||||
selected.push(skill.clone());
|
||||
@@ -196,10 +199,6 @@ fn select_skills_from_mentions(
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_host_skill_path(path: &str) -> String {
|
||||
normalize_skill_path(path).replace('\\', "/")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "selection_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use super::*;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_absolute_path::test_support::test_path_buf;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -9,8 +8,8 @@ use std::collections::HashSet;
|
||||
#[derive(Default)]
|
||||
struct TestLookup {
|
||||
skills: Vec<SkillMetadata>,
|
||||
disabled_paths: HashSet<AbsolutePathBuf>,
|
||||
skill_discovery_path_by_path: HashMap<AbsolutePathBuf, AbsolutePathBuf>,
|
||||
disabled_paths: HashSet<PathUri>,
|
||||
skill_discovery_path_by_path: HashMap<PathUri, PathUri>,
|
||||
}
|
||||
|
||||
impl ExplicitSkillLookup for TestLookup {
|
||||
@@ -18,11 +17,11 @@ impl ExplicitSkillLookup for TestLookup {
|
||||
&self.skills
|
||||
}
|
||||
|
||||
fn disabled_paths(&self) -> &HashSet<AbsolutePathBuf> {
|
||||
fn disabled_paths(&self) -> &HashSet<PathUri> {
|
||||
&self.disabled_paths
|
||||
}
|
||||
|
||||
fn skill_discovery_path_for_path(&self, path: &AbsolutePathBuf) -> Option<&AbsolutePathBuf> {
|
||||
fn skill_discovery_path_for_path(&self, path: &PathUri) -> Option<&PathUri> {
|
||||
self.skill_discovery_path_by_path.get(path)
|
||||
}
|
||||
}
|
||||
@@ -35,7 +34,7 @@ fn make_skill(name: &str, path: &str) -> SkillMetadata {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
path_to_skills_md: test_path_buf(path).abs(),
|
||||
path_to_skills_md: test_path_buf(path).abs().into(),
|
||||
scope: codex_protocol::protocol::SkillScope::User,
|
||||
plugin_id: None,
|
||||
remote_plugin_id: None,
|
||||
@@ -49,7 +48,7 @@ fn linked_skill_mention(name: &str, unix_path: &str) -> String {
|
||||
fn collect_mentions(
|
||||
inputs: &[UserInput],
|
||||
skills: &[SkillMetadata],
|
||||
disabled_paths: &HashSet<AbsolutePathBuf>,
|
||||
disabled_paths: &HashSet<PathUri>,
|
||||
connector_slug_counts: &HashMap<String, usize>,
|
||||
) -> Vec<SkillMetadata> {
|
||||
let loaded_skills = TestLookup {
|
||||
@@ -64,7 +63,7 @@ fn skill_outcome_with_discovery_path(skill: SkillMetadata, discovery_path: &str)
|
||||
TestLookup {
|
||||
skill_discovery_path_by_path: HashMap::from([(
|
||||
skill.path_to_skills_md.clone(),
|
||||
test_path_buf(discovery_path).abs(),
|
||||
test_path_buf(discovery_path).abs().into(),
|
||||
)]),
|
||||
skills: vec![skill],
|
||||
..Default::default()
|
||||
@@ -200,7 +199,7 @@ fn collect_explicit_skill_mentions_skips_disabled_structured_and_blocks_plain_fa
|
||||
path: test_path_buf("/tmp/alpha"),
|
||||
},
|
||||
];
|
||||
let disabled = HashSet::from([test_path_buf("/tmp/alpha").abs()]);
|
||||
let disabled = HashSet::from([test_path_buf("/tmp/alpha").abs().into()]);
|
||||
let connector_counts = HashMap::new();
|
||||
|
||||
let selected = collect_mentions(&inputs, &skills, &disabled, &connector_counts);
|
||||
@@ -298,7 +297,7 @@ fn collect_explicit_skill_mentions_skips_when_linked_path_disabled() {
|
||||
text: format!("use {}", linked_skill_mention("demo-skill", "/tmp/alpha")),
|
||||
text_elements: Vec::new(),
|
||||
}];
|
||||
let disabled = HashSet::from([test_path_buf("/tmp/alpha").abs()]);
|
||||
let disabled = HashSet::from([test_path_buf("/tmp/alpha").abs().into()]);
|
||||
let connector_counts = HashMap::new();
|
||||
|
||||
let selected = collect_mentions(&inputs, &skills, &disabled, &connector_counts);
|
||||
@@ -352,3 +351,82 @@ fn collect_explicit_skill_mentions_skips_missing_path_without_fallback() {
|
||||
|
||||
assert_eq!(selected, Vec::new());
|
||||
}
|
||||
|
||||
/// Linked native paths use URI identity across Windows case and separator spellings.
|
||||
#[test]
|
||||
fn collect_explicit_skill_mentions_matches_windows_path_identity() {
|
||||
let skill = SkillMetadata {
|
||||
path_to_skills_md: PathUri::parse("file:///C:/Skills/Demo/SKILL.md").unwrap(),
|
||||
..make_skill("demo-skill", "/tmp/demo/SKILL.md")
|
||||
};
|
||||
let loaded_skills = TestLookup {
|
||||
skill_discovery_path_by_path: HashMap::from([(
|
||||
skill.path_to_skills_md.clone(),
|
||||
PathUri::parse("file:///C:/Project/.agents/skills/Demo/SKILL.md").unwrap(),
|
||||
)]),
|
||||
skills: vec![skill.clone()],
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
for path in [
|
||||
r"c:\skills\demo\skill.md",
|
||||
"C:/SKILLS/DEMO/skill.md",
|
||||
"skill://c:/project/.agents/skills/demo/skill.md",
|
||||
] {
|
||||
let inputs = vec"),
|
||||
text_elements: Vec::new(),
|
||||
}];
|
||||
|
||||
let selected = collect_explicit_skill_mentions(&inputs, &loaded_skills, &HashMap::new());
|
||||
|
||||
assert_eq!(selected, vec![skill.clone()]);
|
||||
}
|
||||
}
|
||||
|
||||
/// A differently cased disabled Windows identity still blocks a linked skill selection.
|
||||
#[test]
|
||||
fn collect_explicit_skill_mentions_rejects_disabled_windows_path_identity() {
|
||||
let skill = SkillMetadata {
|
||||
path_to_skills_md: PathUri::parse("file:///C:/Skills/Demo/SKILL.md").unwrap(),
|
||||
..make_skill("demo-skill", "/tmp/demo/SKILL.md")
|
||||
};
|
||||
let loaded_skills = TestLookup {
|
||||
skills: vec![skill],
|
||||
disabled_paths: HashSet::from([PathUri::parse("file:///C:/skills/demo/skill.md").unwrap()]),
|
||||
..Default::default()
|
||||
};
|
||||
let inputs = vec".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}];
|
||||
|
||||
let selected = collect_explicit_skill_mentions(&inputs, &loaded_skills, &HashMap::new());
|
||||
|
||||
assert_eq!(selected, Vec::new());
|
||||
}
|
||||
|
||||
/// Native mention text retains literal spaces, percent escapes, and fragments in skill paths.
|
||||
#[test]
|
||||
fn collect_explicit_skill_mentions_preserves_native_uri_characters() {
|
||||
let percent = make_skill("demo-skill", "/tmp/demo skill%23/SKILL.md");
|
||||
let fragment = make_skill("demo-skill", "/tmp/demo skill#/SKILL.md");
|
||||
let loaded_skills = TestLookup {
|
||||
skills: vec![percent.clone(), fragment.clone()],
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
for (path, skill) in [
|
||||
("/tmp/demo skill%23/SKILL.md", percent),
|
||||
("/tmp/demo skill#/SKILL.md", fragment),
|
||||
] {
|
||||
let inputs = vec![UserInput::Text {
|
||||
text: linked_skill_mention("demo-skill", path),
|
||||
text_elements: Vec::new(),
|
||||
}];
|
||||
|
||||
let selected = collect_explicit_skill_mentions(&inputs, &loaded_skills, &HashMap::new());
|
||||
|
||||
assert_eq!(selected, vec![skill]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2976,7 +2976,7 @@ impl App {
|
||||
self.chat_widget.update_skill_enabled(path, enabled);
|
||||
}
|
||||
Err(err) => {
|
||||
let path_display = path.display();
|
||||
let path_display = path.inferred_native_path_string();
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Failed to update skill config for {path_display}: {err}"
|
||||
));
|
||||
|
||||
@@ -44,6 +44,7 @@ use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_approval_presets::ApprovalPreset;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use strum_macros::IntoStaticStr;
|
||||
use uuid::Uuid;
|
||||
|
||||
@@ -1463,7 +1464,7 @@ pub(crate) enum AppEvent {
|
||||
|
||||
/// Enable or disable a skill by path.
|
||||
SetSkillEnabled {
|
||||
path: AbsolutePathBuf,
|
||||
path: PathUri,
|
||||
enabled: bool,
|
||||
},
|
||||
|
||||
|
||||
@@ -4125,7 +4125,7 @@ impl ChatComposer {
|
||||
description,
|
||||
insert_text: format!("${skill_name}"),
|
||||
search_terms,
|
||||
path: Some(skill.path.to_string_lossy().into_owned()),
|
||||
path: Some(skill.path.as_str().to_owned()),
|
||||
category_tag: Some("[Skill]".to_string()),
|
||||
sort_rank: 1,
|
||||
});
|
||||
@@ -6847,7 +6847,7 @@ mod tests {
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
path: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(),
|
||||
path: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs().into(),
|
||||
scope: crate::test_support::skill_scope_user(),
|
||||
enabled: true,
|
||||
plugin_id: None,
|
||||
@@ -7499,7 +7499,7 @@ mod tests {
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
path: skill_path.clone(),
|
||||
path: skill_path.clone().into(),
|
||||
scope: crate::test_support::skill_scope_user(),
|
||||
enabled: true,
|
||||
plugin_id: None,
|
||||
@@ -7544,7 +7544,7 @@ mod tests {
|
||||
default_prompt: None,
|
||||
}),
|
||||
dependencies: None,
|
||||
path: skill_path.clone(),
|
||||
path: skill_path.clone().into(),
|
||||
scope: crate::test_support::skill_scope_repo(),
|
||||
enabled: true,
|
||||
plugin_id: Some("google-calendar@debug".to_string()),
|
||||
@@ -7879,7 +7879,9 @@ mod tests {
|
||||
default_prompt: None,
|
||||
}),
|
||||
dependencies: None,
|
||||
path: test_path_buf("/tmp/repo/google-calendar/SKILL.md").abs(),
|
||||
path: test_path_buf("/tmp/repo/google-calendar/SKILL.md")
|
||||
.abs()
|
||||
.into(),
|
||||
scope: crate::test_support::skill_scope_repo(),
|
||||
enabled: true,
|
||||
plugin_id: None,
|
||||
|
||||
@@ -80,7 +80,7 @@ fn skill_candidate(skill: &SkillMetadata) -> Candidate {
|
||||
mention_type: MentionType::Skill,
|
||||
selection: Selection::Tool {
|
||||
insert_text: format!("${skill_name}"),
|
||||
path: Some(skill.path.to_string_lossy().into_owned()),
|
||||
path: Some(skill.path.as_str().to_owned()),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3563,7 +3563,7 @@ mod tests {
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
path: test_path_buf("/tmp/test-skill/SKILL.md").abs(),
|
||||
path: test_path_buf("/tmp/test-skill/SKILL.md").abs().into(),
|
||||
scope: crate::test_support::skill_scope_user(),
|
||||
enabled: true,
|
||||
plugin_id: None,
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyModifiers;
|
||||
@@ -25,6 +24,7 @@ use crate::render::renderable::ColumnRenderable;
|
||||
use crate::render::renderable::Renderable;
|
||||
use crate::skills_helpers::match_skill;
|
||||
use crate::style::user_message_style;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
|
||||
use super::CancellationEvent;
|
||||
use super::bottom_pane_view::BottomPaneView;
|
||||
@@ -40,7 +40,7 @@ pub(crate) struct SkillsToggleItem {
|
||||
pub skill_name: String,
|
||||
pub description: String,
|
||||
pub enabled: bool,
|
||||
pub path: AbsolutePathBuf,
|
||||
pub path: PathUri,
|
||||
}
|
||||
|
||||
pub(crate) struct SkillsToggleView {
|
||||
@@ -441,14 +441,18 @@ mod tests {
|
||||
skill_name: "polish:superpowers-systematic-debugging".to_string(),
|
||||
description: "Find root causes before fixing bugs".to_string(),
|
||||
enabled: true,
|
||||
path: test_path_buf("/tmp/skills/systematic-debugging/SKILL.md").abs(),
|
||||
path: test_path_buf("/tmp/skills/systematic-debugging/SKILL.md")
|
||||
.abs()
|
||||
.into(),
|
||||
},
|
||||
SkillsToggleItem {
|
||||
name: "superpowers-verification-before-completion (polish)".to_string(),
|
||||
skill_name: "polish:superpowers-verification-before-completion".to_string(),
|
||||
description: "Verify completion before claiming success".to_string(),
|
||||
enabled: false,
|
||||
path: test_path_buf("/tmp/skills/verification-before-completion/SKILL.md").abs(),
|
||||
path: test_path_buf("/tmp/skills/verification-before-completion/SKILL.md")
|
||||
.abs()
|
||||
.into(),
|
||||
},
|
||||
]
|
||||
}
|
||||
@@ -463,14 +467,16 @@ mod tests {
|
||||
skill_name: "repo_scout".to_string(),
|
||||
description: "Summarize the repo layout".to_string(),
|
||||
enabled: true,
|
||||
path: test_path_buf("/tmp/skills/repo_scout.toml").abs(),
|
||||
path: test_path_buf("/tmp/skills/repo_scout.toml").abs().into(),
|
||||
},
|
||||
SkillsToggleItem {
|
||||
name: "Changelog Writer".to_string(),
|
||||
skill_name: "changelog_writer".to_string(),
|
||||
description: "Draft release notes".to_string(),
|
||||
enabled: false,
|
||||
path: test_path_buf("/tmp/skills/changelog_writer.toml").abs(),
|
||||
path: test_path_buf("/tmp/skills/changelog_writer.toml")
|
||||
.abs()
|
||||
.into(),
|
||||
},
|
||||
];
|
||||
let view = SkillsToggleView::new(items, tx, crate::keymap::RuntimeKeymap::defaults().list);
|
||||
|
||||
@@ -603,7 +603,7 @@ pub(crate) struct ChatWidget {
|
||||
pending_collab_spawn_requests: HashMap<String, multi_agents::SpawnRequestSummary>,
|
||||
suppressed_exec_calls: HashSet<String>,
|
||||
skills_all: Vec<SkillMetadata>,
|
||||
skills_initial_state: Option<HashMap<AbsolutePathBuf, bool>>,
|
||||
skills_initial_state: Option<HashMap<PathUri, bool>>,
|
||||
last_unified_wait: Option<UnifiedExecWaitState>,
|
||||
unified_exec_wait_streak: Option<UnifiedExecWaitStreak>,
|
||||
turn_lifecycle: TurnLifecycleState,
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use super::*;
|
||||
use codex_app_server_protocol::ImageReference;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
|
||||
impl ChatWidget {
|
||||
pub(crate) fn set_task_mentions_enabled(&mut self, enabled: bool) {
|
||||
@@ -291,7 +292,7 @@ impl ChatWidget {
|
||||
.map(|binding| binding.mention.clone())
|
||||
.collect();
|
||||
let mut skill_names_lower: HashSet<String> = HashSet::new();
|
||||
let mut selected_skill_paths: HashSet<AbsolutePathBuf> = HashSet::new();
|
||||
let mut selected_skill_paths = HashSet::new();
|
||||
let mut selected_plugin_ids: HashSet<String> = HashSet::new();
|
||||
|
||||
if let Some(skills) = self.bottom_pane.skills() {
|
||||
@@ -300,32 +301,40 @@ impl ChatWidget {
|
||||
.map(|skill| skill.name.to_ascii_lowercase())
|
||||
.collect();
|
||||
|
||||
// TODO(anp): Preserve executor path identity through both structured skill input paths
|
||||
// below and selection. UserInput::Skill still relies on host-native path interpretation.
|
||||
for binding in &mention_bindings {
|
||||
let path = binding
|
||||
.path
|
||||
.strip_prefix("skill://")
|
||||
.unwrap_or(binding.path.as_str());
|
||||
let path = Path::new(path);
|
||||
if let Some(skill) = skills.iter().find(|skill| skill.path.as_path() == path)
|
||||
&& selected_skill_paths.insert(skill.path.clone())
|
||||
let Some(path) = LegacyAppPathString::from_string(path).to_inferred_path_uri()
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
if let Some(skill) = skills
|
||||
.iter()
|
||||
.find(|skill| skill.path.to_inferred_path_uri().as_ref() == Some(&path))
|
||||
&& selected_skill_paths.insert(path)
|
||||
{
|
||||
items.push(UserInput::Skill {
|
||||
name: skill.name.clone(),
|
||||
path: skill.path.to_path_buf(),
|
||||
path: PathBuf::from(skill.path.as_str()),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let skill_mentions = find_skill_mentions_with_tool_mentions(&mentions, skills);
|
||||
for skill in skill_mentions {
|
||||
if bound_names.contains(skill.name.as_str())
|
||||
|| !selected_skill_paths.insert(skill.path.clone())
|
||||
{
|
||||
let Some(path) = skill.path.to_inferred_path_uri() else {
|
||||
continue;
|
||||
};
|
||||
if bound_names.contains(skill.name.as_str()) || !selected_skill_paths.insert(path) {
|
||||
continue;
|
||||
}
|
||||
items.push(UserInput::Skill {
|
||||
name: skill.name.clone(),
|
||||
path: skill.path.to_path_buf(),
|
||||
path: PathBuf::from(skill.path.as_str()),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +19,8 @@ use codex_app_server_protocol::SkillsListResponse;
|
||||
use codex_connectors::AppInfo;
|
||||
use codex_protocol::parse_command::ParsedCommand;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL;
|
||||
|
||||
impl ChatWidget {
|
||||
@@ -64,27 +66,23 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
let mut initial_state = HashMap::new();
|
||||
let mut items = Vec::with_capacity(self.skills_all.len());
|
||||
for skill in &self.skills_all {
|
||||
initial_state.insert(skill.path.clone(), skill.enabled);
|
||||
let Some(path) = skill.path.to_inferred_path_uri() else {
|
||||
tracing::warn!(path = %skill.path, "skill has no absolute path for configuration");
|
||||
continue;
|
||||
};
|
||||
initial_state.insert(path.clone(), skill.enabled);
|
||||
items.push(SkillsToggleItem {
|
||||
name: skill_display_name(skill),
|
||||
skill_name: skill.name.clone(),
|
||||
description: skill_description(skill).to_string(),
|
||||
enabled: skill.enabled,
|
||||
path,
|
||||
});
|
||||
}
|
||||
self.skills_initial_state = Some(initial_state);
|
||||
|
||||
let items: Vec<SkillsToggleItem> = self
|
||||
.skills_all
|
||||
.iter()
|
||||
.map(|skill| {
|
||||
let display_name = skill_display_name(skill);
|
||||
let description = skill_description(skill).to_string();
|
||||
SkillsToggleItem {
|
||||
name: display_name,
|
||||
skill_name: skill.name.clone(),
|
||||
description,
|
||||
enabled: skill.enabled,
|
||||
path: skill.path.clone(),
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
|
||||
let view = SkillsToggleView::new(
|
||||
items,
|
||||
self.app_event_tx.clone(),
|
||||
@@ -93,9 +91,9 @@ impl ChatWidget {
|
||||
self.bottom_pane.show_view(Box::new(view));
|
||||
}
|
||||
|
||||
pub(crate) fn update_skill_enabled(&mut self, path: AbsolutePathBuf, enabled: bool) {
|
||||
pub(crate) fn update_skill_enabled(&mut self, path: PathUri, enabled: bool) {
|
||||
for skill in &mut self.skills_all {
|
||||
if skill.path == path {
|
||||
if skill.path.to_inferred_path_uri().as_ref() == Some(&path) {
|
||||
skill.enabled = enabled;
|
||||
}
|
||||
}
|
||||
@@ -108,7 +106,9 @@ impl ChatWidget {
|
||||
};
|
||||
let mut current_state = HashMap::new();
|
||||
for skill in &self.skills_all {
|
||||
current_state.insert(skill.path.clone(), skill.enabled);
|
||||
if let Some(path) = skill.path.to_inferred_path_uri() {
|
||||
current_state.insert(path, skill.enabled);
|
||||
}
|
||||
}
|
||||
|
||||
let mut enabled_count = 0;
|
||||
@@ -158,10 +158,11 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
// Best effort only: annotate exact SKILL.md path matches from the loaded skills list.
|
||||
if let Some(skill) = self
|
||||
.skills_all
|
||||
.iter()
|
||||
.find(|skill| skill.path.as_path() == path)
|
||||
if let Some(path) = LegacyAppPathString::from_path(path).to_inferred_path_uri()
|
||||
&& let Some(skill) = self
|
||||
.skills_all
|
||||
.iter()
|
||||
.find(|skill| skill.path.to_inferred_path_uri().as_ref() == Some(&path))
|
||||
{
|
||||
*name = format!("{name} ({} skill)", skill.name);
|
||||
}
|
||||
@@ -204,35 +205,49 @@ pub(crate) fn find_skill_mentions_with_tool_mentions(
|
||||
mentions: &ToolMentions,
|
||||
skills: &[SkillMetadata],
|
||||
) -> Vec<SkillMetadata> {
|
||||
let mention_skill_paths: HashSet<&str> = mentions
|
||||
if mentions.names.is_empty() && mentions.linked_paths.is_empty() {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let mention_skill_paths: HashSet<PathUri> = mentions
|
||||
.linked_paths
|
||||
.values()
|
||||
.filter(|path| is_skill_path(path))
|
||||
.map(|path| normalize_skill_path(path))
|
||||
.filter_map(|path| {
|
||||
LegacyAppPathString::from_string(normalize_skill_path(path)).to_inferred_path_uri()
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Share parsed identities between passes while keeping linked paths ahead of plain names.
|
||||
let parsed_skills = skills
|
||||
.iter()
|
||||
.filter_map(|skill| {
|
||||
let path = skill.path.to_inferred_path_uri()?;
|
||||
Some((skill, path))
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let mut seen_names = HashSet::new();
|
||||
let mut seen_paths = HashSet::new();
|
||||
let mut matches: Vec<SkillMetadata> = Vec::new();
|
||||
|
||||
for skill in skills {
|
||||
if seen_paths.contains(&skill.path) {
|
||||
for (skill, path) in &parsed_skills {
|
||||
if seen_paths.contains(path) {
|
||||
continue;
|
||||
}
|
||||
let path_str = skill.path.to_string_lossy();
|
||||
if mention_skill_paths.contains(path_str.as_ref()) {
|
||||
seen_paths.insert(skill.path.clone());
|
||||
if mention_skill_paths.contains(path) {
|
||||
seen_paths.insert(path.clone());
|
||||
seen_names.insert(skill.name.clone());
|
||||
matches.push(skill.clone());
|
||||
matches.push((*skill).clone());
|
||||
}
|
||||
}
|
||||
|
||||
for skill in skills {
|
||||
if seen_paths.contains(&skill.path) {
|
||||
for (skill, path) in parsed_skills {
|
||||
if seen_paths.contains(&path) {
|
||||
continue;
|
||||
}
|
||||
if mentions.names.contains(&skill.name) && seen_names.insert(skill.name.clone()) {
|
||||
seen_paths.insert(skill.path.clone());
|
||||
seen_paths.insert(path);
|
||||
matches.push(skill.clone());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -967,7 +967,7 @@ async fn submission_prefers_selected_duplicate_skill_path() {
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
path: repo_skill_path,
|
||||
path: repo_skill_path.into(),
|
||||
scope: crate::test_support::skill_scope_repo(),
|
||||
enabled: true,
|
||||
plugin_id: None,
|
||||
@@ -978,7 +978,7 @@ async fn submission_prefers_selected_duplicate_skill_path() {
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
path: user_skill_path.clone(),
|
||||
path: user_skill_path.clone().into(),
|
||||
scope: crate::test_support::skill_scope_user(),
|
||||
enabled: true,
|
||||
plugin_id: None,
|
||||
|
||||
@@ -1608,9 +1608,7 @@ pub(super) fn plugins_test_detail(
|
||||
description: format!("{name} description"),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
path: Some(plugins_test_absolute_path(&format!(
|
||||
"skills/{name}/SKILL.md"
|
||||
))),
|
||||
path: Some(plugins_test_absolute_path(&format!("skills/{name}/SKILL.md")).into()),
|
||||
enabled: true,
|
||||
})
|
||||
.collect(),
|
||||
|
||||
@@ -74,13 +74,13 @@ async fn async_question_answers_preserve_ambiguous_skill_selection_and_dismiss_r
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
path: test_path_buf("/tmp/route/SKILL.md").abs(),
|
||||
path: test_path_buf("/tmp/route/SKILL.md").abs().into(),
|
||||
scope: crate::test_support::skill_scope_repo(),
|
||||
enabled: true,
|
||||
plugin_id: None,
|
||||
};
|
||||
let mut duplicate = skill.clone();
|
||||
duplicate.path = test_path_buf("/tmp/other-route/SKILL.md").abs();
|
||||
duplicate.path = test_path_buf("/tmp/other-route/SKILL.md").abs().into();
|
||||
chat.set_skills(Some(vec![skill.clone(), duplicate]));
|
||||
chat.add_async_questions("message", &questions());
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT));
|
||||
@@ -99,7 +99,7 @@ async fn async_question_answers_preserve_ambiguous_skill_selection_and_dismiss_r
|
||||
},
|
||||
UserInput::Skill {
|
||||
name: skill.name,
|
||||
path: skill.path.to_path_buf()
|
||||
path: PathBuf::from(skill.path.as_str())
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
@@ -31,6 +31,7 @@ use codex_protocol::config_types::TrustLevel;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
use codex_utils_path_uri::PathConvention;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use color_eyre::eyre::Result;
|
||||
use color_eyre::eyre::WrapErr;
|
||||
use serde_json::Value as JsonValue;
|
||||
@@ -394,9 +395,12 @@ pub(crate) async fn read_remote_project_trust(
|
||||
|
||||
pub(crate) async fn write_skill_enabled(
|
||||
request_handle: AppServerRequestHandle,
|
||||
path: AbsolutePathBuf,
|
||||
path: PathUri,
|
||||
enabled: bool,
|
||||
) -> Result<()> {
|
||||
// TODO(anp): Support foreign skill paths through skills/config/write and persisted config.
|
||||
// Both still require host-native paths, so retain that restriction here.
|
||||
let path = path.to_abs_path()?;
|
||||
let request_id = RequestId::String(format!("tui-skill-config-write-{}", Uuid::new_v4()));
|
||||
let _: SkillsConfigWriteResponse = request_handle
|
||||
.request_typed(ClientRequest::SkillsConfigWrite {
|
||||
|
||||
@@ -4906,7 +4906,7 @@ class SkillSummary(BaseModel):
|
||||
enabled: bool
|
||||
interface: SkillInterface | None = None
|
||||
name: str
|
||||
path: AbsolutePathBuf | None = None
|
||||
path: LegacyAppPathString | None = None
|
||||
short_description: Annotated[str | None, Field(alias="shortDescription")] = None
|
||||
|
||||
|
||||
@@ -9794,7 +9794,7 @@ class SkillMetadata(BaseModel):
|
||||
enabled: bool
|
||||
interface: SkillInterface | None = None
|
||||
name: str
|
||||
path: AbsolutePathBuf
|
||||
path: LegacyAppPathString
|
||||
plugin_id: Annotated[
|
||||
str | None,
|
||||
Field(
|
||||
|
||||
Reference in New Issue
Block a user