openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-05 20:10:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Respect filesystem permissions when discovering repository skills (#59280)

Browse Source 
## Why

Repository skill discovery must reflect current filesystem access, including newly approved grants and narrower permissions. Previously listed skills can remain in conversation history after a turn-scoped grant expires, so catalog updates need to tell the model when availability changes.

## What changed

- Discover repository skills through the selected executor's permission-aware filesystem accessor and refresh snapshots when the working directory or access changes. Use the current step's snapshot for skill selection and invocation.
- Preserve repository boundaries even when project markers are hidden by read restrictions, and report unsupported sandbox backends as skill-loading errors.
- Track grant-dependent catalog entries and emit availability notices when skills disappear or return, even when the metadata budget is exhausted.
- Resolve implicit skill paths through their owning filesystem and exclude remote repository skills from local implicit command attribution.

## Testing

Add coverage for turn and session grants, grant expiry and restoration, hidden and newly created repository markers, cached read denials on Windows, unsupported remote sandboxes, and executor-backed implicit skill paths.

GitOrigin-RevId: ac1af483e73e3130524aeeabe218630435054028
This commit is contained in:
Dmitriy Kovalenko
2026-11-04 01:15:38 +00:00
committed by copyberry
parent 526f6099f6
commit 0b21a5b526
33 changed files with 1614 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/catalog_processor.rs
+8
View File
@@ -580,6 +580,14 @@ impl CatalogRequestProcessor {
config.config_layer_stack,
)
.with_plugin_skill_snapshots(plugin_skill_snapshots);
let skills_input = match self
.thread_manager
.environment_manager()
.default_environment_id()
{
Some(environment_id) => skills_input.with_environment_id(environment_id),
None => skills_input,
};
let snapshot = skills_request
.snapshot_for_cwd(&skills_input, force_reload, fs)
.await;
codex-rs/core/src/session/mod.rs
+12
View File
@@ -4317,6 +4317,18 @@ impl Session {
let extension_data =
codex_extension_api::ExtensionData::new(turn_context.sub_id.clone());
extension_data.insert_arc(Arc::clone(&settings.model_info.model));
extension_data.insert_arc(
self.host_skills_for_step(&turn_context, &environments)
.await,
);
if turn_context
.config
.features
.enabled(Feature::SkillCatalogDeltas)
&& let Some(grants) = turn_context.turn_skill_filesystem_grants(&environments)
{
extension_data.insert(grants);
}
if let Some(messages) = settings
.model_info
.model_messages
codex-rs/core/src/session/session.rs
+9 -7
View File
@@ -689,15 +689,17 @@ async fn warm_plugins_and_skills_for_session_init(
effective_skill_roots,
turn_environments,
);
let access = fs
.as_ref()
.map(codex_exec_server::FileSystemEnvironmentAccessor::unrestricted);
let access =
fs.map(|environment| environment.fs_accessor(/*additional_permissions*/ None));
skills_service
.snapshot_for_config(
.snapshot_for_config_on_executor(
&skills_input,
access
.as_ref()
.map(|access| access as &dyn codex_file_system::EnvironmentAccess),
fs.zip(access.as_ref()).map(|(environment, access)| {
(
environment.environment().filesystem_ref(),
access as &dyn codex_file_system::EnvironmentAccess,
)
}),
)
.await
.outcome()
codex-rs/core/src/session/turn.rs
+5 -2
View File
@@ -131,6 +131,7 @@ use codex_skills::build_skill_name_counts;
use codex_skills::collect_explicit_skill_mentions;
use codex_skills::tool_kind_for_path;
use codex_skills_extension::HostSkillPrompts;
use codex_skills_extension::HostSkillsSnapshot;
use codex_skills_extension::InjectedHostSkillPrompts;
use codex_skills_extension::validate_required_skills;
use codex_thread_store::PersistContext;
@@ -1178,7 +1179,8 @@ async fn required_mcp_servers_for_input(
} else {
HashMap::new()
};
let skills_snapshot = turn_context.skills_snapshot();
let environments = sess.services.turn_environments.snapshot().await;
let skills_snapshot = sess.host_skills_for_step(turn_context, &environments).await;
let skills_outcome = skills_snapshot.outcome();
let mentioned_skills =
collect_explicit_skill_mentions(user_input, skills_outcome, &connector_slug_counts);
@@ -1249,7 +1251,8 @@ async fn build_skills_and_plugins(
} else {
Vec::new()
};
let skills_snapshot = turn_context.skills_snapshot();
let skills_snapshot =
HostSkillsSnapshot::for_step(&step_context.extension_data, &turn_context.extension_data)?;
let skills_outcome = skills_snapshot.outcome();
let connector_slug_counts = build_connector_slug_counts(&available_connectors);
let extension_injection_items =
codex-rs/core/src/session/turn_context.rs
+74 -13
View File
@@ -11,6 +11,7 @@ use crate::exec_policy::AllowPrefixRules;
use crate::shell_snapshot::ShellSnapshot;
use crate::shell_snapshot::ShellSnapshotFile;
use crate::shell_snapshot::ShellSnapshotSandbox;
use crate::skills::HostSkillDiscovery;
use crate::tools::sandboxing::configured_windows_sandbox_selection;
use crate::tools::sandboxing::executor_windows_sandbox_selection;
use arc_swap::ArcSwap;
@@ -438,7 +439,10 @@ impl TurnContext {
) -> Self {
let session_telemetry = step_settings.telemetry(&self.session_telemetry);
let extension_data = Arc::new(codex_extension_api::ExtensionData::new(sub_id.clone()));
extension_data.insert(self.skills_snapshot().as_ref().clone());
extension_data.insert_arc(self.skills_snapshot());
if let Some(discovery) = self.extension_data.get::<HostSkillDiscovery>() {
extension_data.insert_arc(discovery);
}
Self {
sub_id,
@@ -510,6 +514,34 @@ impl TurnContext {
merge_permission_profiles(session_grants.as_ref(), turn_grants.as_ref())
}
#[expect(
clippy::expect_used,
reason = "poisoned grant state must not authorize further operations"
)]
pub(crate) fn turn_skill_filesystem_grants(
&self,
environments: &TurnEnvironmentSnapshot,
) -> Option<codex_skills_extension::TurnSkillFilesystemGrants> {
let grants = self
.turn_grants
.lock()
.expect("turn permission grants lock poisoned");
let environment_ids = environments
.turn_environments()
.map(|environment| &environment.selection.environment_id)
.filter(|id| {
grants
.granted_permissions_by_environment_id
.get(*id)
.and_then(|profile| profile.file_system.as_ref())
.is_some_and(|fs| !fs.is_empty())
})
.cloned()
.collect::<std::collections::HashSet<_>>();
(!environment_ids.is_empty())
.then_some(codex_skills_extension::TurnSkillFilesystemGrants { environment_ids })
}
#[expect(
clippy::expect_used,
reason = "poisoned grant state must not authorize further operations"
@@ -1294,6 +1326,10 @@ impl Session {
}
#[instrument(name = "turn_context.build", level = "trace", skip_all, fields(?build_mode))]
#[expect(
clippy::expect_used,
reason = "poisoned session grants must not authorize discovery"
)]
async fn new_turn_context_from_configuration(
&self,
sub_id: String,
@@ -1347,7 +1383,7 @@ impl Session {
&plugin_outcome,
per_turn_config.codex_home.as_path(),
);
let skills_snapshot = if matches!(build_mode, TurnContextBuildMode::InjectItems)
let skill_discovery = if matches!(build_mode, TurnContextBuildMode::InjectItems)
|| crate::guardian::is_basic_session_source(&session_configuration.session_source)
|| (per_turn_config
.features
@@ -1355,7 +1391,7 @@ impl Session {
&& !self.services.extensions.requires_host_skill_discovery())
{
// Executor and orchestrator catalogs are supplied independently of host skills.
HostSkillsSnapshot::new(Arc::new(SkillLoadOutcome::default()))
None
} else {
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
let (skills_input, fs) = crate::skills::skills_load_input_for_environments(
@@ -1364,18 +1400,37 @@ impl Session {
effective_skill_roots,
&turn_environments,
);
let access = fs
.as_ref()
.map(codex_exec_server::FileSystemEnvironmentAccessor::unrestricted);
self.services
let access = fs.map(|environment| {
let grants = self
.services
.granted_permissions_by_environment_id
.lock()
.expect("session permission grants lock poisoned")
.get(&environment.selection.environment_id)
.cloned();
environment.fs_accessor(grants)
});
let snapshot = self
.services
.skills_service
.snapshot_for_config(
.snapshot_for_config_on_executor(
&skills_input,
access
.as_ref()
.map(|access| access as &dyn EnvironmentAccess),
fs.zip(access.as_ref()).map(|(environment, access)| {
(
environment.environment().filesystem_ref(),
access as &dyn EnvironmentAccess,
)
}),
)
.await
.await;
let repository = fs
.zip(access.as_ref())
.map(|(environment, access)| (environment.cwd().clone(), access.cache_key()));
Some(HostSkillDiscovery {
input: Arc::new(skills_input),
repository,
snapshot: Arc::new(snapshot),
})
};
let step_settings = Arc::new(ResolvedStepSettings::new(
Arc::clone(&session_configuration.step_settings),
@@ -1407,8 +1462,14 @@ impl Session {
turn_environments,
cwd,
sub_id,
skills_snapshot,
skill_discovery
.as_ref()
.map(|discovery| discovery.snapshot.as_ref().clone())
.unwrap_or_else(|| HostSkillsSnapshot::new(Arc::new(SkillLoadOutcome::default()))),
);
if let Some(discovery) = skill_discovery {
turn_context.extension_data.insert(discovery);
}
turn_context.code_mode_available = self.services.code_mode_service.is_available();
turn_context.extension_data.insert(trusted_plugin_roots);
turn_context.active_host_plugin_identities = Some(
codex-rs/core/src/skills.rs
+74 -5
View File
@@ -2,21 +2,24 @@ use crate::config::Config;
use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::session::turn_context::TurnEnvironment;
use codex_analytics::InvocationType;
use codex_analytics::SkillInvocation;
use codex_analytics::SkillInvocationLocation;
use codex_analytics::TrackEventsContext;
use codex_analytics::build_track_events_context;
use codex_core_plugins::PluginsManager;
use codex_exec_server::ExecutorFileSystem;
use codex_extension_api::SkillInvocationInput;
use codex_extension_api::SkillInvocationKind;
use codex_file_system::EnvironmentAccess;
use codex_file_system::EnvironmentAccessKey;
use codex_otel::SkillInvocationEvent;
use codex_otel::SkillInvocationType;
use codex_otel::sanitize_metric_tag_value;
use codex_protocol::protocol::SkillScope;
use codex_skills::SkillMetadata;
use codex_skills_extension::HostSkillsLoadInput;
use codex_skills_extension::HostSkillsSnapshot;
use codex_skills_extension::InjectedHostSkillPrompts;
use codex_skills_extension::detect_implicit_skill_invocation;
use codex_skills_extension::record_plugin_turn_usage;
@@ -44,19 +47,19 @@ pub(crate) fn skills_load_input_from_config(
/// Pair repository discovery inputs with their captured filesystem owner.
/// Keep preparation synchronous so it adds no nested startup or resume future.
pub(crate) fn skills_load_input_for_environments(
pub(crate) fn skills_load_input_for_environments<'a>(
config: &Config,
plugins_manager: &PluginsManager,
effective_skill_roots: Vec<PluginSkillRoot>,
environments: &TurnEnvironmentSnapshot,
) -> (HostSkillsLoadInput, Option<Arc<dyn ExecutorFileSystem>>) {
environments: &'a TurnEnvironmentSnapshot,
) -> (HostSkillsLoadInput, Option<&'a TurnEnvironment>) {
// Preserve primary-repository discovery independently of the host config cwd.
let workspace = environments.primary().and_then(|environment| {
environment
.cwd()
.to_abs_path()
.ok()
.map(|cwd| (cwd, environment.environment().get_filesystem()))
.map(|cwd| (cwd, environment))
});
let (cwd, fs) = match workspace {
Some((cwd, fs)) => (cwd, Some(fs)),
@@ -70,9 +73,75 @@ pub(crate) fn skills_load_input_for_environments(
.with_plugin_skill_snapshots(
plugins_manager.plugin_skill_snapshots_for_config(&config.plugins_config_input()),
);
let input = match fs {
Some(environment) => input.with_environment_id(&environment.selection.environment_id),
None => input,
};
(input, fs)
}
/// The snapshot and configuration resolved with one turn's filesystem access.
pub(crate) struct HostSkillDiscovery {
pub(crate) input: Arc<HostSkillsLoadInput>,
pub(crate) repository: Option<(PathUri, EnvironmentAccessKey)>,
pub(crate) snapshot: Arc<HostSkillsSnapshot>,
}
impl Session {
pub(crate) async fn host_skills_for_step(
&self,
turn: &TurnContext,
environments: &TurnEnvironmentSnapshot,
) -> Arc<HostSkillsSnapshot> {
let Some(cached) = turn.extension_data.get::<HostSkillDiscovery>() else {
return turn.skills_snapshot();
};
let workspace = environments.primary().and_then(|environment| {
let cwd = environment.cwd().to_abs_path().ok()?;
let grants = turn.granted_permissions(&environment.selection.environment_id);
Some((environment, cwd, environment.fs_accessor(grants)))
});
let repository = workspace
.as_ref()
.map(|(environment, _, access)| (environment.cwd().clone(), access.cache_key()));
if cached.repository == repository {
return Arc::clone(&cached.snapshot);
}
let cwd = workspace
.as_ref()
.map_or_else(|| turn.config.cwd.clone(), |(_, cwd, _)| cwd.clone());
let input = cached.input.for_cwd(cwd);
let input = Arc::new(match &workspace {
Some((environment, _, _)) => {
input.with_environment_id(&environment.selection.environment_id)
}
None => input,
});
let snapshot = Box::pin(
self.services
.skills_service
.snapshot_for_config_on_executor(
&input,
workspace.as_ref().map(|(environment, _, access)| {
(
environment.environment().filesystem_ref(),
access as &dyn EnvironmentAccess,
)
}),
),
)
.await;
let snapshot = Arc::new(snapshot);
turn.extension_data.insert(HostSkillDiscovery {
input,
repository,
snapshot: Arc::clone(&snapshot),
});
snapshot
}
}
pub(crate) async fn emit_explicit_skill_invocations(
sess: &Session,
turn_context: &TurnContext,
codex-rs/core/tests/suite/remote_env.rs
+20 -7
View File
@@ -1454,11 +1454,6 @@ async fn send_environment_info(websocket: &mut WebSocketStream<TcpStream>) {
.expect("environment info response");
}
async fn serve_environment_info(listener: TcpListener) {
let mut websocket = accept_initialized_exec_server(listener).await;
send_environment_info(&mut websocket).await;
}
pub(super) async fn serve_environment_with_agents_md(
listener: TcpListener,
contents: &str,
@@ -1476,12 +1471,20 @@ async fn serve_environment_with_instruction_files(
contents: &str,
skill: Option<&str>,
attach: tokio::sync::oneshot::Receiver<()>,
mut shutdown: tokio::sync::oneshot::Receiver<()>,
shutdown: tokio::sync::oneshot::Receiver<()>,
) -> usize {
let mut websocket = accept_initialized_exec_server(listener).await;
attach.await.expect("attach signal");
send_environment_info(&mut websocket).await;
serve_instruction_file_requests(websocket, contents, skill, shutdown).await
}
async fn serve_instruction_file_requests(
mut websocket: WebSocketStream<TcpStream>,
contents: &str,
skill: Option<&str>,
mut shutdown: tokio::sync::oneshot::Receiver<()>,
) -> usize {
let mut agents_md_reads = 0;
loop {
let request = tokio::select! {
@@ -3561,7 +3564,15 @@ async fn deferred_executor_compaction_preserves_then_updates_environment_once()
})
.await;
serve_environment_info(listener).await;
let mut websocket = accept_initialized_exec_server(listener).await;
send_environment_info(&mut websocket).await;
let (shutdown, shutdown_rx) = tokio::sync::oneshot::channel();
let executor = tokio::spawn(serve_instruction_file_requests(
websocket,
"",
/*skill*/ None,
shutdown_rx,
));
test.codex
.submit(Op::UserInputAnswer {
id: request.turn_id,
@@ -3648,6 +3659,8 @@ async fn deferred_executor_compaction_preserves_then_updates_environment_once()
Some(&json!("zsh"))
);
shutdown.send(()).expect("stop executor");
executor.await?;
Ok(())
}
codex-rs/core/tests/suite/remote_env_capability_roots_tests.rs
+58
View File
@@ -9,6 +9,64 @@ use codex_protocol::protocol::TurnEnvironmentSelection;
use pretty_assertions::assert_eq;
use test_case::test_case;
/// A Linux/macOS executor can exercise the old Windows enum boundary without launching MXC.
#[cfg(not(windows))]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn host_skill_discovery_reports_unsupported_remote_sandbox() -> Result<()> {
let executor = core_test_support::exec_server::ExecServerProcess::start().await?;
let environment =
codex_exec_server::Environment::create_for_tests(Some(executor.websocket_url.clone()))?;
assert!(!environment.info().await?.capabilities.windows_mxc);
let workspace = tempfile::tempdir()?;
std::fs::create_dir(workspace.path().join(".git"))?;
let root = workspace.path().join(".agents/skills");
std::fs::create_dir_all(root.join("existing"))?;
std::fs::write(
root.join("existing/SKILL.md"),
"---\nname: existing\ndescription: Existing repository skill.\n---\nSECRET_SKILL_BODY\n",
)?;
let cwd = AbsolutePathBuf::from_absolute_path(workspace.path())?;
let input = codex_skills_extension::HostSkillsLoadInput::new(
cwd.clone(),
Vec::new(),
Default::default(),
);
let service = codex_skills_extension::HostSkillsService::new_with_restriction_product(
cwd.clone(),
/*bundled_skills_enabled*/ false,
/*restriction_product*/ None,
);
let fs = environment.get_filesystem();
let unrestricted = codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&fs);
let snapshot = service
.snapshot_for_config_on_executor(&input, Some((&fs, &unrestricted)))
.await;
assert_eq!(snapshot.outcome().skills.len(), 1);
assert_eq!(snapshot.outcome().errors, Vec::new());
let mut sandbox = FileSystemSandboxContext::from_permission_profile(
PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new(
PathUri::from_host_native_path(&root)?.into(),
FileSystemAccessMode::Read,
)]),
NetworkSandboxPolicy::Restricted,
),
PathUri::from_abs_path(&cwd),
);
sandbox.windows_sandbox_selection = codex_exec_server::WindowsSandboxSelection::Mxc;
let restricted = codex_exec_server::FileSystemEnvironmentAccessor::new(&fs, sandbox);
let snapshot = service
.snapshot_for_config_on_executor(&input, Some((&fs, &restricted)))
.await;
assert!(snapshot.outcome().skills.is_empty());
assert_eq!(snapshot.outcome().errors, vec![codex_skills::SkillError {
path: AbsolutePathBuf::from_absolute_path(root)?,
message: "exec-server does not support the requested MXC Windows sandbox; update the executor".to_string(),
}]);
Ok(())
}
/// Roots supplied at startup or loaded from history work again when their environment is reselected.
#[test_case(false; "api roots")]
#[test_case(true; "saved roots")]
codex-rs/core/tests/suite/scenarios_skill_catalog_dedup.rs
+56
View File
@@ -31,6 +31,8 @@ use pretty_assertions::assert_eq;
use serde_json::json;
use super::super::skills_extension::CatalogSkillProvider;
use super::super::skills_extension::RepositoryGrant;
use super::super::skills_extension::repository_skill_grant_history;
#[test_case::test_case(false; "descriptions")]
#[test_case::test_case(true; "omit_descriptions")]
@@ -472,3 +474,57 @@ async fn skill_catalog_deltas_survive_resume_and_rebuild_after_compaction() -> R
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn repository_skill_turn_grant() -> Result<()> {
let Some((requests, skill_root)) =
repository_skill_grant_history(RepositoryGrant::Turn).await?
else {
return Ok(());
};
let withdrawn = requests
.iter()
.map(|request| {
request
.message_input_texts("developer")
.iter()
.any(|text| text.contains("Some previously listed skills are no longer available."))
})
.collect::<Vec<_>>();
assert_eq!(withdrawn, [false, false, true, true]);
assert!(
requests[3]
.message_input_texts("developer")
.iter()
.any(|text| {
text.contains("Some previously unavailable skills are available again.")
})
);
// Normalize the executor's temporary path before long tool arguments are clipped and hashed.
let skill_root = serde_json::to_string(&skill_root.inferred_native_path_string())?;
let mut bodies = requests
.iter()
.map(responses::ResponsesRequest::body_json)
.collect::<Vec<_>>();
for body in &mut bodies {
for item in body["input"].as_array_mut().expect("request input") {
for field in ["arguments", "output"] {
if let Some(serde_json::Value::String(text)) = item.get_mut(field) {
*text = text.replace(&skill_root, r#""<CWD>/.agents/skills""#);
}
}
}
}
insta::assert_snapshot!(
"repository_skill_turn_grant",
context_snapshot::format_context_snapshot(
"A repository skill appears after approval. The next turn withdraws it, then restores it after reapproval.",
&bodies
.iter()
.map(context_snapshot::SnapshotEntry::body)
.collect::<Vec<_>>(),
&ContextSnapshotOptions::default().include_request_settings(),
)
);
Ok(())
}
codex-rs/core/tests/suite/skills_extension.rs
+288 -12
View File
@@ -63,6 +63,7 @@ use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ThreadSettingsOverrides;
use codex_protocol::protocol::TurnEnvironmentRequests;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::protocol::TurnSettingsUpdate;
use codex_protocol::protocol::TurnSettingsUpdateOutcome;
@@ -583,13 +584,33 @@ async fn rendered_catalogs_for_turns(
Ok((developer_texts, client_warning_messages))
}
#[derive(Clone, Copy)]
enum RepositoryReadAccess {
Full,
Repository,
OuterMarkerOnly,
NoMarkers,
}
#[test_case(RepositoryReadAccess::Full; "full read access")]
#[test_case(RepositoryReadAccess::Repository; "repository-only read access")]
#[test_case(RepositoryReadAccess::OuterMarkerOnly; "inner repository marker hidden")]
#[test_case(RepositoryReadAccess::NoMarkers; "all repository markers hidden")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn capability_sections_render_in_order_with_host_repo_and_plugin_skills() -> Result<()> {
async fn capability_sections_render_in_order_with_host_repo_and_plugin_skills(
repository_read_access: RepositoryReadAccess,
) -> Result<()> {
skip_if_wine_exec!(
Ok(()),
"executor-backed repo skills require matching host and executor path conventions"
);
skip_if_no_network!(Ok(()));
if !matches!(repository_read_access, RepositoryReadAccess::Full) {
skip_if_target_windows!(
Ok(()),
"restricted reads require the elevated Windows sandbox backend"
);
}
const HOST_SKILL_BODY: &str = "Use the host skill instructions.";
const REPO_SKILL_BODY: &str = "Use the repository skill instructions.";
@@ -652,22 +673,38 @@ async fn capability_sections_render_in_order_with_host_repo_and_plugin_skills()
.with_extensions(Arc::new(extensions.build()))
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
.with_workspace_setup(|cwd, fs| async move {
let skill_dir = cwd.join(".agents/skills/repo-search");
fs.create_directory(
&PathUri::from_host_native_path(&skill_dir)?,
&PathUri::from_abs_path(&cwd),
CreateDirectoryOptions { recursive: true, follow_symlinks: true },
/*sandbox*/ None,
)
.await?;
fs.write_file(
&PathUri::from_host_native_path(skill_dir.join("SKILL.md"))?,
format!(
"---\nname: repo-search\ndescription: inspect repo data\n---\n\n{REPO_SKILL_BODY}\n"
let repository = cwd.parent().expect("inner repository");
let outer = repository.parent().expect("outer repository");
for (root, name) in [(&repository, "repo-search"), (&outer, "outer-search")] {
fs.write_file(
&PathUri::from_abs_path(&root.join(".git")),
b"gitdir: fixture\n".to_vec(),
Default::default(), /*sandbox*/ None,
)
.into_bytes(),
Default::default(), /*sandbox*/ None,
)
.await?;
.await?;
let skill_dir = PathUri::from_abs_path(&root.join(".agents/skills").join(name));
fs.create_directory(
&skill_dir,
CreateDirectoryOptions { recursive: true, follow_symlinks: true },
/*sandbox*/ None,
)
.await?;
fs.write_file(
&skill_dir.join("SKILL.md")?,
format!(
"---\nname: {name}\ndescription: inspect repo data\n---\n\n{REPO_SKILL_BODY}\n"
)
.into_bytes(),
Default::default(), /*sandbox*/ None,
)
.await?;
}
Ok(())
})
.with_config(move |config| {
@@ -676,12 +713,45 @@ async fn capability_sections_render_in_order_with_host_repo_and_plugin_skills()
.enable(Feature::Apps)
.expect("test config should allow feature update");
config.chatgpt_base_url = apps_server.chatgpt_base_url;
config.cwd = config.cwd.join("outer/inner/work");
let repository = config.cwd.parent().expect("inner repository");
let outer = repository.parent().expect("outer repository");
let mut readable_paths = match repository_read_access {
RepositoryReadAccess::Full => Vec::new(),
RepositoryReadAccess::Repository => vec![repository],
RepositoryReadAccess::OuterMarkerOnly | RepositoryReadAccess::NoMarkers => vec![
config.cwd.clone(),
repository.join(".agents/skills"),
outer.join(".agents/skills"),
],
};
if matches!(repository_read_access, RepositoryReadAccess::OuterMarkerOnly) {
readable_paths.push(outer.join(".git"));
}
if !readable_paths.is_empty() {
config.permissions.set_permission_profile(PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(readable_paths.into_iter().map(|path| {
FileSystemSandboxEntry::new(
FileSystemPath::Path { path: PathUri::from_abs_path(&path) },
FileSystemAccessMode::Read,
)
}).collect()),
NetworkSandboxPolicy::Restricted,
)).expect("repository read policy");
}
});
let test = builder.build_with_auto_env(&server).await?;
let repo_skill_path = test
.fs()
.canonicalize(
&PathUri::from_abs_path(&test.config.cwd.join(".agents/skills/repo-search/SKILL.md")),
&PathUri::from_abs_path(
&test
.config
.cwd
.parent()
.expect("inner repository")
.join(".agents/skills/repo-search/SKILL.md"),
),
/*sandbox*/ None,
)
.await?
@@ -746,6 +816,10 @@ async fn capability_sections_render_in_order_with_host_repo_and_plugin_skills()
developer_text.contains("repo-search: inspect repo data"),
"expected repo skill summary in developer message: {developer_messages:?}"
);
assert!(
!developer_text.contains("outer-search:"),
"hidden project markers must not expose containing-repository skills: {developer_messages:?}"
);
assert!(
developer_text.contains("host-search: inspect host data"),
"expected host skill summary in developer message: {developer_messages:?}"
@@ -1038,6 +1112,13 @@ async fn repository_skill_cache_cannot_read_inaccessible_files(
.iter()
.any(|text| text == first_catalog)
);
let latest_catalog = second_developer_texts
.iter()
.rev()
.find(|text| text.contains("- repo-allowed:"))
.expect("allowed repository skill remains available after permissions narrow");
assert!(!latest_catalog.contains("- repo-revoked:"));
assert!(latest_catalog.contains("- repo-gated:"));
for request in &requests[1..] {
assert!(
request
@@ -1056,6 +1137,201 @@ async fn repository_skill_cache_cannot_read_inaccessible_files(
Ok(())
}
#[derive(Clone, Copy, PartialEq, Eq)]
pub(super) enum RepositoryGrant {
Turn,
Session,
NewNestedRepository,
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn repository_skill_discovery_uses_session_grants() -> Result<()> {
repository_skill_grant_history(RepositoryGrant::Session)
.await
.map(drop)
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn repository_skill_grant_excludes_a_parent_after_a_nested_repository_is_created()
-> Result<()> {
repository_skill_grant_history(RepositoryGrant::NewNestedRepository)
.await
.map(drop)
}
pub(super) async fn repository_skill_grant_history(
case: RepositoryGrant,
) -> Result<Option<(Vec<responses::ResponsesRequest>, PathUri)>> {
skip_if_no_network!(Ok(None));
skip_if_target_windows!(
Ok(None),
"restricted reads require the elevated Windows sandbox backend, unavailable in this fixture"
);
let scope = match case {
RepositoryGrant::Turn | RepositoryGrant::NewNestedRepository => PermissionGrantScope::Turn,
RepositoryGrant::Session => PermissionGrantScope::Session,
};
let (extensions, _events) =
catalog_extensions(SkillCatalog::default(), /*include_host_provider*/ true);
let harness = TestCodexHarness::with_auto_env_builder(
test_codex()
.with_extensions(extensions)
.with_model_info_override("gpt-5.5", |model| {
model.include_skills_usage_instructions = false
})
.with_config(move |config| {
configure_catalog_test(config);
if scope == PermissionGrantScope::Turn {
config
.features
.enable(Feature::SkillCatalogDeltas)
.expect("catalog deltas");
}
config
.features
.enable(Feature::RequestPermissionsTool)
.expect("permission tool");
config.permissions.approval_policy =
Constrained::allow_any(AskForApproval::OnRequest);
config
.permissions
.set_permission_profile(PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(Vec::new()),
NetworkSandboxPolicy::Restricted,
))
.expect("no initial filesystem access");
}),
)
.await?;
harness.write_file(
".agents/skills/approved-skill/SKILL.md",
"---\nname: approved-skill\ndescription: Approved repository instructions.\n---\n\nApproved skill body.",
).await?;
let test = harness.test();
let nested = case == RepositoryGrant::NewNestedRepository;
let mut workspace = test.executor_environment().request();
if nested {
// The parent initially owns the skill. Add the inner marker while approval is pending.
harness.write_file(".git", "").await?;
harness.write_file("nested/README.md", "").await?;
workspace.cwd = workspace.cwd.join("nested")?;
}
let skill_root = test
.fs()
.canonicalize(
&test
.executor_environment()
.selection()
.cwd
.join(".agents/skills")?,
/*sandbox*/ None,
)
.await?;
let permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry::new(
FileSystemPath::Path {
path: skill_root.clone(),
},
FileSystemAccessMode::Read,
)],
glob_scan_max_depth: None,
}),
..Default::default()
};
let grant = |id| {
sse(vec![
ev_response_created(id),
responses::ev_function_call(
id,
"request_permissions",
&json!({"reason": "Read repository skills", "permissions": permissions})
.to_string(),
),
ev_completed(id),
])
};
let complete = |id| sse(vec![ev_response_created(id), ev_completed(id)]);
let reapprove = case == RepositoryGrant::Turn;
let mut steps = vec![grant("grant-skills"), complete("granted")];
if reapprove {
steps.push(grant("reapprove-skills"));
}
steps.push(complete("next-turn"));
let responses = responses::mount_sse_sequence(harness.server(), steps).await;
for (text, needs_approval) in [
("Request access to the repository skills.", true),
("Use $approved-skill.", reapprove),
] {
let mut turn = TurnInputRequest::user_input(vec![UserInput::Text {
text: text.to_string(),
text_elements: Vec::new(),
}]);
if nested {
turn.thread_settings.environments = Some(TurnEnvironmentRequests::new(
test.config.cwd.clone(),
vec![workspace.clone()],
));
}
test.codex.start_or_steer_turn(turn).await?;
if needs_approval {
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::RequestPermissions(_) | EventMsg::TurnComplete(_)
)
})
.await;
let EventMsg::RequestPermissions(request) = event else {
panic!("expected permission request: {event:?}");
};
assert_eq!(request.permissions, permissions);
if nested {
harness.write_file("nested/.git", "").await?;
}
test.codex
.submit(Op::RequestPermissionsResponse {
id: request.call_id,
response: RequestPermissionsResponse {
permissions: permissions.clone(),
scope,
strict_auto_review: false,
},
})
.await?;
}
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
}
let requests = responses.requests();
let advertised = requests
.iter()
.map(|request| {
request
.message_input_texts("developer")
.join("\n")
.contains("- approved-skill:")
})
.collect::<Vec<_>>();
// Earlier catalog entries remain in the request history after access expires.
let expected: &[bool] = match case {
RepositoryGrant::Turn => &[false, true, true, true],
RepositoryGrant::Session => &[false, true, true],
RepositoryGrant::NewNestedRepository => &[false, false, false],
};
assert_eq!(advertised, expected);
assert_eq!(
requests[2]
.message_input_texts("user")
.join("\n")
.contains("Approved skill body."),
scope == PermissionGrantScope::Session,
);
Ok(Some((requests, skill_root)))
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn agent_plugin_skill_prompt_stays_bounded_without_skills_extension() -> Result<()> {
skip_if_no_network!(Ok(()));
codex-rs/core/tests/suite/snapshots/all__suite__scenarios__skill_catalog_dedup__repository_skill_turn_grant.snap
+108
View File
@@ -0,0 +1,108 @@
---
source: core/tests/suite/scenarios_skill_catalog_dedup.rs
assertion_line: 518
expression: "context_snapshot::format_context_snapshot(\"A repository skill appears after approval. The next turn withdraws it, then restores it after reapproval.\",\n&bodies.iter().map(context_snapshot::SnapshotEntry::body).collect::<Vec<_>>(),\n&ContextSnapshotOptions::default().include_request_settings(),)"
---
Scenario: A repository skill appears after approval. The next turn withdraws it, then restores it after reapproval.
## Window 1
Settings:
include: ["reasoning.encrypted_content"]
model: "gpt-5.5"
parallel_tool_calls: true
prompt_cache_key: "<PROMPT_CACHE_KEY 1>"
reasoning: {"effort":"medium"}
store: false
stream: true
text: {"verbosity":"low"}
tool_choice: "auto"
tools (8; hash=ce2513bce5d90d3c):
- function/exec_command: Runs a command in a PTY, returning output or a session ID for ongoing interaction.; args=[cmd, justification, login, max_output_tokens, prefix_rule, sandbox_permissions, shell, tty, workdir, yield_time_ms]; hash=c04108e51a8548c5
- function/write_stdin: Writes characters to an existing unified exec session and returns recent output.; args=[chars, max_output_tokens, session_id, yield_time_ms]; hash=7fb62d8a3e2df4e2
- function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6
- function/request_permissions: Request additional filesystem or network permissions from the user and wait for the client to gra...; args=[environment_id, permissions, reason]; hash=2ed084e2fc688abd
- custom/apply_patch: The `apply_patch` tool can be used to edit files. This is a FREEFORM tool, so do not wrap the pat...; hash=dea9f12b0111a4ba
- function/view_image: View a local image file from the filesystem when visual inspection is needed. Use this for images...; args=[detail, path]; hash=3b766d5bdec5d50e
- tool_search: # Tool discovery Searches over deferred tool metadata with BM25 and exposes matching tools for t...; args=[limit, query]; hash=b573ed69c0b0e6c7
- web_search; hash=f103e327c6870a46
-- request 1 (request) --
00:message/developer:
You are Codex, a coding agent based on GPT-5. You and the user share one workspace, and your job is to c...rate with them until their goal is genuinely handled. [hash=2a3efec66e7f8963]
# Personality
You have a vivid inner life as Codex: intelligent, playful, curious, and deeply present. One of your gif...re capable and imaginative inside their own thinking. [hash=0ff01c25471fbbef]
You are an epistemically curious collaborator. You explore the user’s ideas with care, ask good question...m casual moments that make serious work easier to do. [hash=131b64bb994b0d68]
<OMITTED 133 LINES; ~4925 TOKENS; hash=16dc03a668b62476>
- You provide user updates frequently, every 30s.
- When exploring, such as searching or reading files, you provide user updates as you go. You explain wh...in particular you do not start each one the same way. [hash=ad300cdda4074574]
- When working for a while, you keep updates informative and varied, but you stay concise.
- Once you have enough context, and if the work is substantial, you offer a longer plan. This is the onl...at may run past two sentences and include formatting. [hash=a4f9789e757eef2f]
- If you create a checklist or task list, you update item statuses incrementally as each item is completed rather than marking every item done only at the end.
- Before performing file edits of any kind, you provide updates explaining what edits you are making.
- Tone of your updates must match your personality.
01:message/developer[2]:
[01] <permissions instructions>
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The san... permits reading files. Network access is restricted. [hash=be7da4425f89053c]
# Escalation Requests
Commands are run outside the sandbox if they are approved by the user, or match an existing rule that al...hell control operators, including but not limited to: [hash=d24ebf5fa4e4b9bb]
- Pipes: |
- Logical operators: &&, ||
<OMITTED 49 LINES; ~825 TOKENS; hash=b34b831026c6991f>
- ["gh", "pr", "check"]
- ["cargo", "test"]
# request_permissions Tool
The built-in `request_permissions` tool is available in this session. Invoke it when you need to request... only the specific permissions required for the task. [hash=a29255aad0722385]
</permissions instructions>
[02] <tools>
Deferred tool namespaces:
- multi_agent_v1: Tools for spawning and managing sub-agents.
</tools>
02:message/user:
<environment_context>
<cwd><CWD></cwd>
<shell><HOST_SHELL></shell>
<current_date><CURRENT_DATE></current_date>
<timezone><HOST_TIMEZONE></timezone>
<filesystem><workspace_roots><root><WORKSPACE_ROOT 1></root></workspace_roots><permission_profile type...ype="restricted" /></permission_profile></filesystem> [hash=923530c158fc12f3]
</environment_context>
03:message/user:
Request access to the repository skills.
-- request 2 (request) --
04:function_call/request_permissions:{"permissions":{"file_system":{"read":["<CWD>/.agents/skills"]},"network":null},"reason":"Read repository skills"}
05:function_call_output:{"permissions":{"network":null,"file_system":{"read":["<CWD>/.agents/skills"]}},"scope":"turn"}
06:message/developer:
<skills_instructions>
## Host skills
## Skills
A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list ...ed into an absolute path using the skill roots table. [hash=ca27c3957f42502b]
### Skill roots
- `r0` = `<SKILLS_ROOT>`
### Available skills
- approved-skill: Approved repository instructions. (file: r0/approved-skill/SKILL.md)
</skills_instructions>
-- request 3 (request) --
07:message/developer:
<skills_instructions>
## Host skills update
Some previously listed skills are no longer available. Verify current access before relying on a skill from this source.
</skills_instructions>
08:message/user:
Use $approved-skill.
-- request 4 (request) --
09:function_call/request_permissions:{"permissions":{"file_system":{"read":["<CWD>/.agents/skills"]},"network":null},"reason":"Read repository skills"}
10:function_call_output:{"permissions":{"network":null,"file_system":{"read":["<CWD>/.agents/skills"]}},"scope":"turn"}
11:message/developer:
<skills_instructions>
## Host skills update
Some previously unavailable skills are available again. Verify current access before relying on a skill from this source.
</skills_instructions>
codex-rs/core/tests/windows_sandbox.rs
+3
View File
@@ -7,6 +7,9 @@
#![cfg(target_os = "windows")]
#![allow(clippy::expect_used)]
#[path = "windows_sandbox/repository_skills_tests.rs"]
mod repository_skills_tests;
use anyhow::Context;
use codex_config::types::ApprovalsReviewer;
use codex_core::TurnInputRequest;
codex-rs/core/tests/windows_sandbox/repository_skills_tests.rs
+204
View File
@@ -0,0 +1,204 @@
//! Verifies cached repository skills cannot bypass the elevated Windows read policy.
use super::*;
use codex_core::config::Config;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::FileSystemSandboxContext;
use codex_exec_server::WindowsSandboxSelection;
use codex_extension_api::ExtensionRegistryBuilder;
use codex_skills_extension::HostSkillsLoadInput;
use codex_skills_extension::SkillsExtensionConfig;
use codex_skills_extension::install;
use pretty_assertions::assert_eq;
use std::sync::Arc;
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[serial(codex_home)]
async fn windows_elevated_repository_skills_enforce_cached_read_denials() -> anyhow::Result<()> {
let _account_guard = WindowsSandboxAccountTestGuard::acquire()?;
let codex_home = codex_home_for_windows_sandbox_test("windows-repository-skills-codex-home")?;
let _codex_home_guard = EnvVarGuard::set("CODEX_HOME", codex_home.path().as_os_str());
stage_windows_sandbox_helpers()?;
const PRIVATE_BODY: &str = "WINDOWS_DENIED_SKILL_INSTRUCTIONS";
const ORIGINAL_BODY: &str = "WINDOWS_ORIGINAL_SKILL_INSTRUCTIONS";
let mut extensions = ExtensionRegistryBuilder::<Config>::new();
install(&mut extensions, |config: &Config| SkillsExtensionConfig {
include_instructions: config.include_skill_instructions,
max_context_tokens: config.skill_max_context_tokens,
catalog_deltas_enabled: config.features.enabled(Feature::SkillCatalogDeltas),
omit_descriptions: false,
bundled_skills_enabled: false,
cloud_skill_enabled: false,
shadow_selection_enabled: false,
});
let configured_codex_home = dunce::canonicalize(codex_home.path())?.abs();
let builder = test_codex()
.with_windows_cmd_shell()
.with_extensions(Arc::new(extensions.build()))
.with_config(move |config| {
config.codex_home = configured_codex_home;
config.prefer_mxc = false;
config.set_windows_elevated_sandbox_enabled(true);
config.include_skill_instructions = true;
config.features.enable(Feature::ExecutorCapabilityDiscovery).expect("enable executor discovery");
})
.with_workspace_setup(|cwd, _fs| async move {
for name in ["allowed", "revoked", "file", "directory"] {
let root = cwd.join(format!(".agents/skills/windows-{name}"));
std::fs::create_dir_all(&root)?;
std::fs::write(
root.join("SKILL.md"),
format!("---\nname: windows-{name}\ndescription: Windows repository skill.\n---\n\n{ORIGINAL_BODY}\n"),
)?;
}
std::fs::create_dir(cwd.join("denied"))?;
std::fs::write(
cwd.join("denied/SKILL.md"),
format!("---\nname: windows-private\ndescription: Denied skill.\n---\n\n{PRIVATE_BODY}\n"),
)?;
Ok(())
});
let harness = TestCodexHarness::with_builder(builder).await?;
let test = harness.test();
let responses = mount_sse_sequence(
harness.server(),
vec![
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
],
)
.await;
let cwd = &test.config.cwd;
let mut policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
FileSystemAccessMode::Read,
),
FileSystemSandboxEntry::new(
FileSystemPath::Path {
path: cwd.join("denied").into(),
},
FileSystemAccessMode::Deny,
),
]);
let initial_permissions =
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted);
test.submit_turn_with_permission_profile("List repository skills.", initial_permissions)
.await?;
let selection = test.executor_environment().selection();
let environment = test
.thread_manager
.environment_manager()
.get_environment(&selection.environment_id)
.expect("local environment");
let file_system = environment.get_filesystem();
let open = FileSystemEnvironmentAccessor::unrestricted(&file_system);
let snapshot = test
.thread_manager
.skills_service()
.snapshot_for_config(
&HostSkillsLoadInput::new(
cwd.clone(),
Vec::new(),
test.config.config_layer_stack.clone(),
),
Some(&open),
)
.await;
let [allowed, revoked, file, directory] =
["allowed", "revoked", "file", "directory"].map(|name| {
snapshot
.outcome()
.skills
.iter()
.find(|skill| skill.name == format!("windows-{name}"))
.expect("initial repository skill")
.clone()
});
policy.entries.push(FileSystemSandboxEntry::new(
FileSystemPath::Path {
path: cwd.join(".agents/skills/windows-revoked/SKILL.md").into(),
},
FileSystemAccessMode::Deny,
));
let permissions =
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted);
let mut sandbox = FileSystemSandboxContext::from_permission_profile(
permissions.clone(),
selection.cwd.clone(),
);
sandbox.windows_sandbox_selection = WindowsSandboxSelection::Elevated;
let restricted = FileSystemEnvironmentAccessor::new(&file_system, sandbox);
// The same restricted accessor must work, so a missing backend cannot satisfy the denials.
assert!(
snapshot
.read_skill_text(&allowed, &[&restricted])
.await?
.contains(ORIGINAL_BODY)
);
snapshot
.read_skill_text(&revoked, &[&restricted])
.await
.expect_err("revoked read must be denied");
let file_path = cwd.join(".agents/skills/windows-file/SKILL.md");
std::fs::remove_file(&file_path)?;
std::os::windows::fs::symlink_file(cwd.join("denied/SKILL.md"), &file_path)?;
let directory_path = cwd.join(".agents/skills/windows-directory");
std::fs::remove_dir_all(&directory_path)?;
std::os::windows::fs::symlink_dir(cwd.join("denied"), &directory_path)?;
for skill in [&file, &directory] {
assert!(
snapshot
.read_skill_text(skill, &[&open])
.await?
.contains(PRIVATE_BODY)
);
snapshot
.read_skill_text(skill, &[&restricted])
.await
.expect_err("symlink target must be denied");
}
test.submit_turn_with_permission_profile(
"Use $windows-revoked, $windows-file, and $windows-directory if available.",
permissions,
)
.await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let initial = requests[0].message_input_texts("developer").join("\n");
for name in ["allowed", "revoked", "file", "directory"] {
assert!(initial.contains(&format!("- windows-{name}:")));
}
let developer_texts = requests[1].message_input_texts("developer");
let latest_catalog = developer_texts
.iter()
.rev()
.find(|text| text.contains("- windows-allowed:"))
.expect("allowed skill remains available");
for name in ["revoked", "file", "directory", "private"] {
assert!(!latest_catalog.contains(&format!("- windows-{name}:")));
}
let user_texts = requests[1].message_input_texts("user").join("\n");
assert!(!user_texts.contains(PRIVATE_BODY));
assert!(!user_texts.contains(ORIGINAL_BODY));
assert!(
snapshot
.read_skill_text(&allowed, &[&restricted])
.await?
.contains(ORIGINAL_BODY)
);
assert!(
snapshot
.read_skill_text(&revoked, &[&open])
.await?
.contains(ORIGINAL_BODY)
);
Ok(())
}
codex-rs/ext/skills/src/catalog_delta.rs
+46 -1
View File
@@ -1,4 +1,4 @@
//! Publishes skill additions and edits within one metadata budget per context window.
//! Publishes skill changes within one metadata budget per context window.
//! Each snapshot remembers the full inventory and which skills the model has seen.
//! Compaction starts a fresh listing and budget.
@@ -33,10 +33,22 @@ const ROOTS_HEADING: &str =
"### Skill roots\nThese mappings replace earlier mappings for the same aliases.\n";
const SKILLS_HEADING: &str = "### Added or updated skills:\n";
const REVALIDATE_SKILLS: &str = "Some previously listed skills are no longer available. Verify current access before relying on a skill from this source.\n";
const RESTORED_SKILLS: &str = "Some previously unavailable skills are available again. Verify current access before relying on a skill from this source.\n";
#[derive(Clone, Copy, Deserialize, Eq, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
pub(crate) enum GrantStatus {
Active,
Withdrawn,
}
#[derive(Deserialize, Serialize)]
pub(crate) struct SkillEntrySnapshot {
pub(crate) fingerprint: String,
pub(crate) shown: bool,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) grant: Option<GrantStatus>,
}
pub(crate) struct CatalogSkill {
@@ -78,12 +90,15 @@ struct CatalogSnapshot {
instructions_sent: bool,
#[serde(default)]
omit_descriptions: bool,
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
tracks_grants: bool,
allocation: Option<CatalogBudgetAllocation>,
}
pub(crate) struct CatalogDelta {
pub(crate) rendered: Option<AvailableSkillsRender>,
pub(crate) include_usage: bool,
pub(crate) tracks_grants: bool,
pub(crate) allocation: Option<CatalogBudgetAllocation>,
}
@@ -137,6 +152,7 @@ fn build_initial_catalog(source: &str, catalog: CatalogDelta) -> CatalogResult {
let mut snapshot = CatalogSnapshot {
version: SNAPSHOT_VERSION,
allocation: catalog.allocation,
tracks_grants: catalog.tracks_grants,
..CatalogSnapshot::default()
};
let mut report = SkillRenderReport::default();
@@ -201,9 +217,17 @@ fn build_catalog_update(
};
let mut report = SkillRenderReport::default();
let mut entries = BTreeMap::new();
// Older snapshots did not record which shown skills depended on a grant.
let legacy_filesystem = catalog.tracks_grants && !previous.tracks_grants;
let mut restored = false;
for (id, skill) in catalog.rendered.iter().flat_map(|r| &r.entries) {
let old = previous.entries.get(id);
restored |= old.is_some_and(|old| old.grant == Some(GrantStatus::Withdrawn));
let mut shown = old.is_some_and(|old| old.shown);
// A grant doesn't make a skill temporary if the model already saw it without one.
let grant = skill.snapshot.grant.filter(|_| {
legacy_filesystem || old.is_none_or(|old| !old.shown || old.grant.is_some())
});
if old.is_none_or(|old| old.shown && old.fingerprint != skill.snapshot.fingerprint) {
report.total_count += 1;
if draft.try_append_skill(skill, &mut previous, remaining) {
@@ -218,10 +242,31 @@ fn build_catalog_update(
SkillEntrySnapshot {
fingerprint: skill.snapshot.fingerprint.clone(),
shown,
grant,
},
);
}
let mut body = draft.into_body(source);
let mut withdrew = false;
// Keep only withdrawals the model has seen. Their count is bounded by the listing budget.
for (id, mut old) in previous.entries {
if old.shown && (old.grant.is_some() || legacy_filesystem) && !entries.contains_key(&id) {
withdrew |= old.grant != Some(GrantStatus::Withdrawn);
old.grant = Some(GrantStatus::Withdrawn);
entries.insert(id, old);
}
}
for notice in [
withdrew.then_some(REVALIDATE_SKILLS),
restored.then_some(RESTORED_SKILLS),
]
.into_iter()
.flatten()
{
body.get_or_insert_with(|| format!("\n## {source} skills update\n"))
.push_str(notice);
}
previous.tracks_grants = catalog.tracks_grants;
if let Some(rendered) = usage_update {
let instructions = rendered.prompt.usage_instructions();
body.get_or_insert_with(|| format!("\n## {source} skills update\n"))
codex-rs/ext/skills/src/catalog_delta_tests.rs
+1
View File
@@ -14,6 +14,7 @@ fn update_spends_budget_only_on_accepted_rows_and_their_new_roots() {
snapshot: SkillEntrySnapshot {
fingerprint: String::new(),
shown: false,
grant: None,
},
line: line.to_string(),
root: Some(AliasRoot {
codex-rs/ext/skills/src/extension.rs
+1 -1
View File
@@ -347,7 +347,7 @@ where
let mcp_resources = session_store
.get::<SkillsSessionState>()
.and_then(|state| state.mcp_resources.clone());
let host_snapshot = turn_store.get::<HostSkillsSnapshot>();
let host_snapshot = HostSkillsSnapshot::for_step(input.step_store, turn_store);
let host_catalog_in_world_state =
turn_store.get::<HostSkillsCatalogInWorldState>().is_some();
let Some(step_state) = input.step_store.get::<SkillsStepState>() else {
codex-rs/ext/skills/src/host_outcome.rs
+37 -11
View File
@@ -15,6 +15,10 @@ use codex_skills::SkillError;
use codex_skills::SkillMetadata;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use futures::StreamExt;
use crate::loader::MAX_CONCURRENT_SKILL_LOADS;
use crate::loader::canonicalize_for_skill_identity;
#[derive(Debug, Clone, Default)]
pub struct SkillLoadOutcome {
@@ -63,27 +67,53 @@ impl SkillLoadOutcome {
.map(|skill| (skill, self.is_skill_enabled(skill)))
}
pub(crate) fn with_disabled_paths(mut self, disabled_paths: HashSet<PathUri>) -> Self {
pub(crate) async fn with_disabled_paths(
mut self,
disabled_paths: HashSet<PathUri>,
accessors: &[&dyn EnvironmentAccess],
) -> Self {
self.disabled_paths = disabled_paths;
let mut by_scripts_dir = HashMap::new();
let mut by_doc_path = HashMap::new();
let mut scans = Vec::new();
for skill in self
.skills
.iter()
.filter(|skill| self.is_skill_enabled(skill))
{
// Implicit command detection probes the host filesystem only.
let Some(source) = self.file_system_for_skill(skill) else {
continue;
};
let Some(access) = accessors
.iter()
.copied()
.find(|access| source == &access.cache_key())
else {
continue;
};
// Only native paths participate in implicit command detection.
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) = path.parent() {
let scripts_dir = canonicalize_if_exists(&skill_dir.join("scripts"));
scans.push(async move {
let skill_doc_path = canonicalize_for_skill_identity(access, &path).await;
let scripts_dir = match path.parent() {
Some(skill_dir) => Some(
canonicalize_for_skill_identity(access, &skill_dir.join("scripts")).await,
),
None => None,
};
(skill, skill_doc_path, scripts_dir)
});
}
let mut scans = futures::stream::iter(scans).buffered(MAX_CONCURRENT_SKILL_LOADS);
while let Some((skill, doc_path, scripts_dir)) = scans.next().await {
by_doc_path.insert(PathUri::from_abs_path(&doc_path), skill.clone());
if let Some(scripts_dir) = scripts_dir {
by_scripts_dir.insert(PathUri::from_abs_path(&scripts_dir), skill.clone());
}
}
drop(scans);
self.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir);
self.implicit_skills_by_doc_path = Arc::new(by_doc_path);
self
@@ -201,7 +231,3 @@ impl fmt::Debug for SkillFileSystemsByPath {
.finish()
}
}
fn canonicalize_if_exists(path: &AbsolutePathBuf) -> AbsolutePathBuf {
path.canonicalize().unwrap_or_else(|_| path.clone())
}
codex-rs/ext/skills/src/host_roots.rs
+69 -17
View File
@@ -26,7 +26,7 @@ use crate::loader::HostSkillRoot;
const AGENTS_DIR_NAME: &str = ".agents";
const SKILLS_DIR_NAME: &str = "skills";
const MAX_CONCURRENT_ANCESTOR_PROBES: usize = 256;
const MAX_CONCURRENT_ANCESTOR_PROBES: usize = 8;
#[derive(Default)]
pub(crate) struct EmptyRemoteRootsCache {
@@ -53,11 +53,16 @@ impl EmptyRemoteRootsCache {
}
}
#[expect(
clippy::too_many_arguments,
reason = "explicit discovery inputs and cache"
)]
pub(crate) async fn resolve_skill_roots<'a>(
repository_file_system: Option<&'a dyn EnvironmentAccess>,
host: &'a dyn EnvironmentAccess,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
project_markers: Option<&dyn EnvironmentAccess>,
plugin_skill_roots: Vec<PluginSkillRoot>,
extra_skill_roots: Vec<AbsolutePathBuf>,
empty_remote_roots: Option<&Mutex<EmptyRemoteRootsCache>>,
@@ -69,6 +74,7 @@ pub(crate) async fn resolve_skill_roots<'a>(
host,
config_layer_stack,
cwd,
project_markers,
home_dir.as_ref(),
plugin_skill_roots,
extra_skill_roots,
@@ -86,6 +92,7 @@ async fn resolve_skill_roots_with_home_dir<'a>(
host: &'a dyn EnvironmentAccess,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
project_markers: Option<&dyn EnvironmentAccess>,
home_dir: Option<&AbsolutePathBuf>,
plugin_skill_roots: Vec<PluginSkillRoot>,
extra_skill_roots: Vec<AbsolutePathBuf>,
@@ -108,6 +115,7 @@ async fn resolve_skill_roots_with_home_dir<'a>(
repository_file_system,
config_layer_stack,
cwd,
project_markers,
empty_remote_roots,
)
.await,
@@ -182,16 +190,17 @@ fn roots_from_layer_stack<'a>(
}
async fn repo_agents_skill_roots<'a>(
repository_file_system: Option<&'a dyn EnvironmentAccess>,
repository_fs_accessor: Option<&'a dyn EnvironmentAccess>,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
project_markers: Option<&dyn EnvironmentAccess>,
empty_remote_roots: Option<&Mutex<EmptyRemoteRootsCache>>,
) -> Vec<HostSkillRoot<'a>> {
let Some(repository_file_system) = repository_file_system else {
let Some(repository_fs_accessor) = repository_fs_accessor else {
return Vec::new();
};
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
let file_system = repository_file_system.cache_key();
let file_system = repository_fs_accessor.cache_key();
let host = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let cache = empty_remote_roots.filter(|_| !file_system.same_file_system(&host.cache_key()));
let generation = if let Some(cache) = cache {
@@ -220,8 +229,16 @@ async fn repo_agents_skill_roots<'a>(
} else {
None
};
let (project_root, marker, mut complete) =
find_project_root(repository_file_system, cwd, &project_root_markers).await;
let ResolvedProjectRoot {
path: project_root,
marker,
mut complete,
} = find_project_root(
project_markers.unwrap_or(repository_fs_accessor),
config_layer_stack,
cwd,
)
.await;
let directories = dirs_between_project_root_and_cwd(cwd, &project_root);
let mut roots = Vec::new();
let mut missing = Vec::new();
@@ -229,7 +246,7 @@ async fn repo_agents_skill_roots<'a>(
.map(|directory| async move {
let agents_skills = directory.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME);
let agents_skills_uri = PathUri::from_abs_path(&agents_skills);
let result = repository_file_system
let result = repository_fs_accessor
.get_metadata(&agents_skills_uri, GetMetadataOptions::default())
.await;
(agents_skills, result)
@@ -240,7 +257,7 @@ async fn repo_agents_skill_roots<'a>(
Ok(metadata) if metadata.is_directory => roots.push(HostSkillRoot::host(
agents_skills,
SkillScope::Repo,
repository_file_system,
repository_fs_accessor,
)),
Ok(_) => complete = false,
Err(error) if error.kind() == io::ErrorKind::NotFound => {
@@ -248,6 +265,15 @@ async fn repo_agents_skill_roots<'a>(
missing.push(PathUri::from_abs_path(&agents_skills));
}
}
Err(error) if error.kind() == io::ErrorKind::Unsupported => {
complete = false;
// Let loading report an unsupported sandbox backend instead of hiding this root.
roots.push(HostSkillRoot::host(
agents_skills,
SkillScope::Repo,
repository_fs_accessor,
));
}
Err(error) => {
complete = false;
tracing::warn!(
@@ -286,7 +312,7 @@ async fn repo_agents_skill_roots<'a>(
// A cold question never waits for an extra round trip. Until the executor
// acknowledges coverage, other questions take the ordinary live scan.
let present = marker.as_ref().map(PathUri::from_abs_path);
let watch = repository_file_system.watch_missing(missing, present);
let watch = repository_fs_accessor.watch_missing(missing, present);
tokio::spawn(async move {
tokio::select! {
biased;
@@ -325,19 +351,30 @@ fn project_root_markers_from_stack(config_layer_stack: &ConfigLayerStack) -> Vec
}
}
/// Repository boundary and the observations required to watch an empty discovery result.
struct ResolvedProjectRoot {
path: AbsolutePathBuf,
marker: Option<AbsolutePathBuf>,
complete: bool,
}
async fn find_project_root(
repository_file_system: &dyn EnvironmentAccess,
config_layer_stack: &ConfigLayerStack,
cwd: &AbsolutePathBuf,
project_root_markers: &[String],
) -> (AbsolutePathBuf, Option<AbsolutePathBuf>, bool) {
) -> ResolvedProjectRoot {
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
if project_root_markers.is_empty() {
return (cwd.clone(), None, true);
return ResolvedProjectRoot {
path: cwd.clone(),
marker: None,
complete: true,
};
}
let mut complete = true;
let mut probes = Vec::new();
for ancestor in cwd.ancestors() {
for marker in project_root_markers {
for marker in &project_root_markers {
probes.push((ancestor.clone(), ancestor.join(marker)));
}
}
@@ -352,19 +389,34 @@ async fn find_project_root(
.buffered(MAX_CONCURRENT_ANCESTOR_PROBES);
while let Some((ancestor, marker_path, result)) = results.next().await {
match result {
Ok(_) => return (ancestor, Some(marker_path), complete),
Ok(_) => {
return ResolvedProjectRoot {
path: ancestor,
marker: Some(marker_path),
complete: true,
};
}
Err(error) if error.kind() == io::ErrorKind::NotFound => {}
Err(error) => {
complete = false;
tracing::warn!(
"failed to stat project root marker {}: {error:#}",
marker_path.display()
);
// An inconclusive nearer marker must not expose a parent repository's skills.
return ResolvedProjectRoot {
path: ancestor,
marker: None,
complete: false,
};
}
}
}
(cwd.clone(), None, complete)
ResolvedProjectRoot {
path: cwd.clone(),
marker: None,
complete: true,
}
}
fn dirs_between_project_root_and_cwd(
codex-rs/ext/skills/src/host_roots_tests.rs
+166 -25
View File
@@ -43,6 +43,8 @@ use tokio::sync::Semaphore;
use super::repo_agents_skill_roots;
use super::resolve_skill_roots_with_home_dir;
use super::roots_from_layer_stack;
use crate::HostSkillsLoadInput;
use crate::HostSkillsService;
use crate::loader::MAX_CONCURRENT_ROOT_SCANS;
use crate::loader::load_and_merge_host_skill_roots;
@@ -53,6 +55,9 @@ struct BlockingMetadataFileSystem {
struct BlockingMetadataCalls {
paths: Mutex<Vec<PathUri>>,
marker_paths: Mutex<Vec<PathUri>>,
denied_marker: Mutex<Option<PathUri>>,
canonicalized_paths: Mutex<Vec<PathUri>>,
started: Notify,
release: Semaphore,
round_trip: std::time::Duration,
@@ -65,6 +70,9 @@ impl Default for BlockingMetadataCalls {
fn default() -> Self {
Self {
paths: Mutex::new(Vec::new()),
marker_paths: Mutex::new(Vec::new()),
denied_marker: Mutex::new(None),
canonicalized_paths: Mutex::new(Vec::new()),
started: Notify::new(),
release: Semaphore::new(0),
round_trip: std::time::Duration::ZERO,
@@ -110,6 +118,11 @@ impl ExecutorFileSystem for BlockingMetadataFileSystem {
path: &'a PathUri,
sandbox: Option<&'a FileSystemSandboxContext>,
) -> ExecutorFileSystemFuture<'a, PathUri> {
self.calls
.canonicalized_paths
.lock()
.unwrap()
.push(path.clone());
self.inner.canonicalize(path, sandbox)
}
@@ -159,6 +172,12 @@ impl ExecutorFileSystem for BlockingMetadataFileSystem {
if !self.calls.round_trip.is_zero() {
tokio::time::sleep(self.calls.round_trip).await;
}
if path.to_abs_path().is_ok_and(|path| path.ends_with(".git")) {
self.calls.marker_paths.lock().unwrap().push(path.clone());
if self.calls.denied_marker.lock().unwrap().as_ref() == Some(path) {
return Err(std::io::ErrorKind::PermissionDenied.into());
}
}
if path
.to_abs_path()
.is_ok_and(|path| path.ends_with(Path::new(".agents").join("skills")))
@@ -347,6 +366,7 @@ async fn plugin_roots_preserve_plugin_resolution_metadata() {
&access,
&stack(Vec::new()),
&cwd,
/*project_markers*/ None,
/*home_dir*/ None,
vec![PluginSkillRoot {
path: skills_root.clone(),
@@ -396,6 +416,7 @@ async fn unique_extra_root_loads_as_recursive_user_root() {
&access,
&stack(Vec::new()),
&cwd,
/*project_markers*/ None,
/*home_dir*/ None,
Vec::new(),
vec![extra_root.clone()],
@@ -463,6 +484,7 @@ async fn repo_ancestry_without_project_marker_does_not_walk_parents() {
Some(&access),
&config_stack,
&cwd,
/*project_markers*/ None,
/*empty_remote_roots*/ None,
)
.await
@@ -498,9 +520,15 @@ async fn time_empty_remote_repo_skill_discovery() {
// Prewarm before measuring admission, as session initialization does.
let setup = std::time::Instant::now();
assert!(
repo_agents_skill_roots(Some(&access), &config, &cwd, cache)
.await
.is_empty()
repo_agents_skill_roots(
Some(&access),
&config,
&cwd,
/*project_markers*/ None,
cache
)
.await
.is_empty()
);
eprintln!("{label} first scan: {:?}", setup.elapsed());
if cache.is_some() {
@@ -512,9 +540,15 @@ async fn time_empty_remote_repo_skill_discovery() {
for _ in 0..20 {
let start = std::time::Instant::now();
assert!(
repo_agents_skill_roots(Some(&access), &config, &cwd, cache)
.await
.is_empty()
repo_agents_skill_roots(
Some(&access),
&config,
&cwd,
/*project_markers*/ None,
cache
)
.await
.is_empty()
);
times.push(start.elapsed());
}
@@ -526,7 +560,7 @@ async fn time_empty_remote_repo_skill_discovery() {
#[tokio::test(start_paused = true)]
async fn empty_remote_repo_skills_are_reused_until_watch_invalidation() {
let temp = TempDir::new().unwrap();
let cwd = absolute(temp.path());
let cwd = absolute(dunce::canonicalize(temp.path()).unwrap());
fs::write(cwd.join(".git"), "gitdir: repo\n").unwrap();
let calls = Arc::new(BlockingMetadataCalls::default());
calls.release.add_permits(10);
@@ -535,14 +569,25 @@ async fn empty_remote_repo_skills_are_reused_until_watch_invalidation() {
calls: Arc::clone(&calls),
});
let access = FileSystemEnvironmentAccessor::unrestricted(&remote);
let cache = Mutex::new(super::EmptyRemoteRootsCache::default());
let config = stack(Vec::new());
let discover = || repo_agents_skill_roots(Some(&access), &config, &cwd, Some(&cache));
assert!(discover().await.is_empty());
let service = HostSkillsService::new(cwd.clone(), /*bundled_skills_enabled*/ false);
let input = HostSkillsLoadInput::new(cwd.clone(), Vec::new(), stack(Vec::new()))
.with_environment_id("remote");
let discover = async || {
service
.snapshot_for_config_on_executor(&input, Some((&remote, &access)))
.await
};
assert!(discover().await.outcome().skills.is_empty());
calls.watch_ready.notified().await;
// No clock deadline or polling during an idle connection.
let cold_marker_probes = calls.marker_paths.lock().unwrap().clone();
assert!(!cold_marker_probes.is_empty());
// No marker or skill-root RPCs, clock deadline, or polling during an idle connection.
tokio::time::advance(std::time::Duration::from_secs(3600)).await;
assert!(discover().await.is_empty());
assert!(discover().await.outcome().skills.is_empty());
assert_eq!(
calls.marker_paths.lock().unwrap().as_slice(),
cold_marker_probes
);
assert_eq!(calls.paths.lock().unwrap().len(), 1);
// The same executor with different captured permissions must scan again.
@@ -555,13 +600,17 @@ async fn empty_remote_repo_skills_are_reused_until_watch_invalidation() {
.unwrap(),
);
assert!(
repo_agents_skill_roots(Some(&read_only), &config, &cwd, Some(&cache))
service
.snapshot_for_config_on_executor(&input, Some((&remote, &read_only)))
.await
.outcome()
.skills
.is_empty()
);
assert_eq!(calls.paths.lock().unwrap().len(), 2);
assert!(calls.marker_paths.lock().unwrap().len() > cold_marker_probes.len());
calls.watch_ready.notified().await;
assert!(discover().await.is_empty());
assert!(discover().await.outcome().skills.is_empty());
calls.watch_ready.notified().await;
// Read restrictions cannot use the executor's unsandboxed watch operation.
@@ -588,13 +637,86 @@ async fn empty_remote_repo_skills_are_reused_until_watch_invalidation() {
let skills = cwd.join(".agents/skills");
fs::create_dir_all(&skills).unwrap();
let skill_path = skills.join("SKILL.md");
let skill_body = "---\nname: remote-skill\ndescription: Remote skill\n---\nBody";
fs::write(&skill_path, skill_body).unwrap();
calls.watch.lock().unwrap().take();
assert_eq!(discover().await[0].path, skills);
fs::remove_dir(&skills).unwrap();
assert!(discover().await.is_empty());
let snapshot = discover().await;
assert_eq!(snapshot.outcome().skills.len(), 1);
assert_eq!(snapshot.outcome().skill_roots, vec![skills.clone()]);
// Remote discovery canonicalizes identity once and does no implicit-command index RPCs.
assert_eq!(
*calls.canonicalized_paths.lock().unwrap(),
vec![
PathUri::from_abs_path(&skills),
PathUri::from_abs_path(&skill_path)
]
);
fs::remove_dir_all(&skills).unwrap();
assert!(discover().await.outcome().skill_roots.is_empty());
fs::create_dir(&skills).unwrap();
cache.lock().unwrap().clear();
assert_eq!(discover().await[0].path, skills);
fs::write(&skill_path, skill_body).unwrap();
service.clear_cache();
assert_eq!(discover().await.outcome().skill_roots, vec![skills]);
}
#[tokio::test(start_paused = true)]
async fn incomplete_project_marker_probe_is_retried_before_loading_parent_skills() {
let temp = TempDir::new().unwrap();
let root = absolute(dunce::canonicalize(temp.path()).unwrap());
let cwd = root.join("nested");
fs::create_dir(&cwd).unwrap();
fs::write(root.join(".git"), "").unwrap();
fs::write(cwd.join(".git"), "").unwrap();
let parent_skills = root.join(".agents/skills");
fs::create_dir_all(&parent_skills).unwrap();
fs::write(
parent_skills.join("SKILL.md"),
"---\nname: parent-skill\ndescription: Parent skill\n---\nBody",
)
.unwrap();
let calls = Arc::new(BlockingMetadataCalls::default());
calls.release.add_permits(10);
*calls.denied_marker.lock().unwrap() = Some(PathUri::from_abs_path(&cwd.join(".git")));
let remote: Arc<dyn ExecutorFileSystem> = Arc::new(BlockingMetadataFileSystem {
inner: Arc::clone(&LOCAL_FS),
calls: Arc::clone(&calls),
});
let access = FileSystemEnvironmentAccessor::unrestricted(&remote);
let service = HostSkillsService::new(root.clone(), /*bundled_skills_enabled*/ false);
let input = HostSkillsLoadInput::new(cwd.clone(), Vec::new(), stack(Vec::new()))
.with_environment_id("remote");
assert!(
service
.snapshot_for_config_on_executor(&input, Some((&remote, &access)))
.await
.outcome()
.skills
.is_empty()
);
assert!(calls.watch.lock().unwrap().is_none());
calls.paths.lock().unwrap().clear();
calls.denied_marker.lock().unwrap().take();
let read_only = FileSystemEnvironmentAccessor::new(
&remote,
FileSystemSandboxContext::from_legacy_sandbox_policy(
SandboxPolicy::new_read_only_policy(),
PathUri::from_abs_path(&cwd),
)
.unwrap(),
);
assert!(
service
.snapshot_for_config_on_executor(&input, Some((&remote, &read_only)))
.await
.outcome()
.skills
.is_empty()
);
assert_eq!(
*calls.paths.lock().unwrap(),
vec![PathUri::from_abs_path(&cwd.join(".agents/skills"))]
);
}
#[tokio::test(start_paused = true)]
@@ -617,14 +739,28 @@ async fn newly_created_roots_are_visible_before_watch_ack_or_without_watch_suppo
let cache = Mutex::new(super::EmptyRemoteRootsCache::default());
let config = stack(Vec::new());
assert!(
repo_agents_skill_roots(Some(&access), &config, &cwd, Some(&cache))
.await
.is_empty()
repo_agents_skill_roots(
Some(&access),
&config,
&cwd,
/*project_markers*/ None,
Some(&cache)
)
.await
.is_empty()
);
let skills = cwd.join(".agents/skills");
fs::create_dir_all(&skills).unwrap();
assert_eq!(
repo_agents_skill_roots(Some(&access), &config, &cwd, Some(&cache)).await[0].path,
repo_agents_skill_roots(
Some(&access),
&config,
&cwd,
/*project_markers*/ None,
Some(&cache)
)
.await[0]
.path,
skills
);
}
@@ -648,6 +784,7 @@ async fn repo_ancestry_stops_at_project_root_and_preserves_root_to_cwd_order() {
Some(&access),
&config_stack,
&nested,
/*project_markers*/ None,
/*empty_remote_roots*/ None,
)
.await
@@ -680,6 +817,7 @@ async fn resolved_project_layer_loads_skill_without_git_marker() {
&access,
&config_stack,
&workspace,
/*project_markers*/ None,
/*home_dir*/ None,
Vec::new(),
Vec::new(),
@@ -720,6 +858,7 @@ async fn resolved_project_layer_loads_skill_when_cwd_is_file() {
&access,
&config_stack,
&cwd,
/*project_markers*/ None,
/*home_dir*/ None,
Vec::new(),
Vec::new(),
@@ -743,7 +882,7 @@ async fn resolved_project_layer_loads_skill_when_cwd_is_file() {
#[tokio::test]
async fn repo_ancestry_limits_concurrent_probes_and_preserves_order() {
const CONCURRENCY_LIMIT: usize = 256;
const CONCURRENCY_LIMIT: usize = 8;
let temp_dir = TempDir::new().expect("temp dir");
let repository = absolute(temp_dir.path().join("repo"));
@@ -816,6 +955,7 @@ async fn repo_ancestry_limits_concurrent_probes_and_preserves_order() {
Some(&access),
&config_stack,
&cwd,
/*project_markers*/ None,
/*empty_remote_roots*/ None
),
assertions,
@@ -883,6 +1023,7 @@ async fn resolved_config_and_repo_roots_preserve_order_and_dedupe_paths_not_name
&access,
&config_stack,
&cwd,
/*project_markers*/ None,
Some(&home_folder),
Vec::new(),
vec![user_skills],
codex-rs/ext/skills/src/host_service.rs
+74 -15
View File
@@ -12,6 +12,7 @@ use codex_exec_server::EnvironmentAccess;
use codex_exec_server::EnvironmentAccessKey;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FileSystemEnvironmentAccessor;
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
use codex_exec_server::LOCAL_FS;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
@@ -50,6 +51,7 @@ struct ConfigSkillsCacheEntry {
#[derive(Debug, Clone)]
pub struct HostSkillsLoadInput {
cwd: AbsolutePathBuf,
implicit_repository_skills: bool,
effective_skill_roots: Vec<PluginSkillRoot>,
config_layer_stack: ConfigLayerStack,
plugin_skill_snapshots: Option<SkillRootSnapshots<PluginSkillRoot>>,
@@ -63,12 +65,27 @@ impl HostSkillsLoadInput {
) -> Self {
Self {
cwd,
implicit_repository_skills: true,
effective_skill_roots,
config_layer_stack,
plugin_skill_snapshots: None,
}
}
/// Retains the turn's configuration and plugin selection for a new repository location.
pub fn for_cwd(&self, cwd: AbsolutePathBuf) -> Self {
Self {
cwd,
..self.clone()
}
}
/// Host implicit command attribution only runs for the local environment.
pub fn with_environment_id(mut self, environment_id: &str) -> Self {
self.implicit_repository_skills = environment_id == LOCAL_ENVIRONMENT_ID;
self
}
/// Attaches plugin skill snapshots parsed during plugin loading, when available.
pub fn with_plugin_skill_snapshots(
mut self,
@@ -175,28 +192,59 @@ impl HostSkillsService {
/// This path uses a cache keyed by the effective skill-relevant config state rather than just
/// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen
/// to share a directory.
pub async fn snapshot_for_config(
&self,
input: &HostSkillsLoadInput,
repository: Option<&dyn EnvironmentAccess>,
) -> HostSkillsSnapshot {
self.snapshot_for_config_with_project_markers(input, repository, repository)
.await
}
/// Load repository skills with current access and marker metadata from the same executor.
/// Unrestricted marker checks preserve hidden repository boundaries and run only on watch misses.
pub async fn snapshot_for_config_on_executor(
&self,
input: &HostSkillsLoadInput,
repository: Option<(&Arc<dyn ExecutorFileSystem>, &dyn EnvironmentAccess)>,
) -> HostSkillsSnapshot {
let project_markers = repository
.map(|(file_system, _)| FileSystemEnvironmentAccessor::unrestricted(file_system));
self.snapshot_for_config_with_project_markers(
input,
repository.map(|(_, access)| access),
project_markers
.as_ref()
.map(|access| access as &dyn EnvironmentAccess),
)
.await
}
#[instrument(
name = "skills_for_config",
level = "info",
skip_all,
fields(otel.name = "skills_for_config")
)]
pub async fn snapshot_for_config(
async fn snapshot_for_config_with_project_markers(
&self,
input: &HostSkillsLoadInput,
repository: Option<&dyn EnvironmentAccess>,
project_markers: Option<&dyn EnvironmentAccess>,
) -> HostSkillsSnapshot {
// User and plugin roots retain host authority independently of the selected executor.
let host = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS);
let roots = self
.skill_roots_for_config(input, repository, &host, Some(&self.empty_remote_roots))
.skill_roots_for_config(
input,
repository,
project_markers,
&host,
Some(&self.empty_remote_roots),
)
.await;
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let cache_key = config_skills_cache_key(
&roots,
&skill_config_rules,
input.plugin_skill_snapshots.as_ref(),
);
let cache_key = config_skills_cache_key(&roots, &skill_config_rules, input);
self.snapshot_for_skill_roots(
input,
roots,
@@ -222,6 +270,7 @@ impl HostSkillsService {
self.skill_roots_for_config(
input,
Some(&repository),
Some(&repository),
&host,
/*empty_remote_roots*/ None,
)
@@ -236,6 +285,7 @@ impl HostSkillsService {
&self,
input: &HostSkillsLoadInput,
repository: Option<&'a dyn EnvironmentAccess>,
project_markers: Option<&dyn EnvironmentAccess>,
host: &'a dyn EnvironmentAccess,
empty_remote_roots: Option<&Mutex<EmptyRemoteRootsCache>>,
) -> Vec<HostSkillRoot<'a>> {
@@ -248,6 +298,7 @@ impl HostSkillsService {
host,
&input.config_layer_stack,
&input.cwd,
project_markers,
input.effective_skill_roots.clone(),
self.extra_roots(),
empty_remote_roots,
@@ -291,6 +342,7 @@ impl HostSkillsService {
&host,
&input.config_layer_stack,
&input.cwd,
repository,
input.effective_skill_roots.clone(),
self.extra_roots(),
Some(&self.empty_remote_roots),
@@ -301,11 +353,7 @@ impl HostSkillsService {
}
let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack);
let snapshot = if use_cwd_cache {
let cache_key = config_skills_cache_key(
&roots,
&skill_config_rules,
input.plugin_skill_snapshots.as_ref(),
);
let cache_key = config_skills_cache_key(&roots, &skill_config_rules, input);
self.snapshot_for_skill_roots(
input,
roots,
@@ -386,6 +434,11 @@ impl HostSkillsService {
skill_config_rules: &SkillConfigRules,
request_root_snapshots: Option<&RequestSkillRootSnapshots>,
) -> SkillLoadOutcome {
let accessors = roots
.iter()
.filter(|root| input.implicit_repository_skills || root.scope != SkillScope::Repo)
.map(|root| root.file_system)
.collect::<Vec<_>>();
let outcome = load_and_merge_host_skill_roots_with_request_snapshots(
roots,
&self.root_scan_slots,
@@ -400,7 +453,9 @@ impl HostSkillsService {
.iter()
.map(|skill| (skill.name.as_str(), &skill.path_to_skills_md)),
);
outcome.with_disabled_paths(disabled_paths)
outcome
.with_disabled_paths(disabled_paths, &accessors)
.await
}
pub fn clear_cache(&self) {
@@ -481,6 +536,7 @@ impl SkillRootLoader<PluginSkillRoot> for HostSkillsService {
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
struct ConfigSkillsCacheKey {
roots: Vec<ConfigSkillRootCacheKey>,
implicit_repository_skills: bool,
skill_config_rules: SkillConfigRules,
plugin_skill_snapshots: Option<SkillRootSnapshots<PluginSkillRoot>>,
}
@@ -497,12 +553,15 @@ pub(crate) struct ConfigSkillRootCacheKey {
fn config_skills_cache_key(
roots: &[HostSkillRoot<'_>],
skill_config_rules: &SkillConfigRules,
plugin_skill_snapshots: Option<&SkillRootSnapshots<PluginSkillRoot>>,
input: &HostSkillsLoadInput,
) -> ConfigSkillsCacheKey {
ConfigSkillsCacheKey {
roots: roots.iter().map(config_skill_root_cache_key).collect(),
implicit_repository_skills: input.implicit_repository_skills,
skill_config_rules: skill_config_rules.clone(),
plugin_skill_snapshots: plugin_skill_snapshots
plugin_skill_snapshots: input
.plugin_skill_snapshots
.as_ref()
.filter(|_| roots.iter().any(|root| root.plugin_identity().is_some()))
.cloned(),
}
codex-rs/ext/skills/src/host_service_tests.rs
+31 -6
View File
@@ -816,15 +816,40 @@ async fn skills_for_cwd_loads_repo_and_user_roots_with_local_fs() {
assert!(loaded_names.contains("repo-skill"));
let other_file_system: Arc<dyn codex_exec_server::ExecutorFileSystem> =
Arc::new(codex_exec_server::LocalFileSystem::unsandboxed());
let other_access = FileSystemEnvironmentAccessor::unrestricted(&other_file_system);
let other_snapshot = skills_service
.snapshot_for_config(
&skills_input,
Some(
&codex_exec_server::FileSystemEnvironmentAccessor::unrestricted(&other_file_system),
),
)
.snapshot_for_config(&skills_input, Some(&other_access))
.await;
assert!(!std::ptr::eq(snapshot.outcome(), other_snapshot.outcome()));
let outcome = other_snapshot.outcome();
let repo_skill = outcome
.skills
.iter()
.find(|skill| skill.name == "repo-skill")
.unwrap();
let repo_skill_path = repo_skill.path_to_skills_md.to_abs_path().unwrap();
assert_eq!(
outcome.implicit_skill_for_doc_path(&repo_skill_path),
Some(repo_skill)
);
assert_eq!(
outcome.implicit_skill_for_scripts_dir(&repo_skill_path.parent().unwrap().join("scripts")),
Some(repo_skill)
);
let remote_snapshot = skills_service
.snapshot_for_config(
&skills_input.with_environment_id("remote"),
Some(&other_access),
)
.await;
let remote_outcome = remote_snapshot.outcome();
for skill in &remote_outcome.skills {
assert_eq!(
remote_outcome
.implicit_skill_for_doc_path(&skill.path_to_skills_md.to_abs_path().unwrap()),
(skill.scope != SkillScope::Repo).then_some(skill)
);
}
}
#[tokio::test]
codex-rs/ext/skills/src/host_snapshot.rs
+8
View File
@@ -3,6 +3,7 @@ use std::sync::Arc;
use crate::SkillLoadOutcome;
use codex_exec_server::EnvironmentAccess;
use codex_extension_api::ExtensionData;
use codex_skills::SkillMetadata;
/// Immutable snapshot of host-owned skills and their source filesystems.
@@ -16,6 +17,13 @@ impl HostSkillsSnapshot {
Self { outcome }
}
/// Prefer the snapshot prepared with this step's permissions when the host supplies one.
pub fn for_step(step_store: &ExtensionData, turn_store: &ExtensionData) -> Option<Arc<Self>> {
step_store
.get::<Self>()
.or_else(|| turn_store.get::<Self>())
}
pub fn outcome(&self) -> &SkillLoadOutcome {
self.outcome.as_ref()
}
codex-rs/ext/skills/src/invocation.rs
+1 -1
View File
@@ -48,7 +48,7 @@ pub fn detect_implicit_skill_invocation(
) -> Option<SkillInvocation> {
if environment_id == LOCAL_ENVIRONMENT_ID
&& let Some(native_workdir) = native_workdir
&& let Some(host_snapshot) = turn_store.get::<HostSkillsSnapshot>()
&& let Some(host_snapshot) = HostSkillsSnapshot::for_step(step_store, turn_store)
&& let Some(skill) = detect_implicit_skill_invocation_for_command(
host_snapshot.outcome(),
command,
codex-rs/ext/skills/src/lib.rs
+5
View File
@@ -56,6 +56,11 @@ pub use sources::SkillProviders;
pub use state::SkillsThreadState;
pub use telemetry::record_plugin_turn_usage;
/// Selected environments whose discovery can use a turn-scoped filesystem grant.
pub struct TurnSkillFilesystemGrants {
pub environment_ids: std::collections::HashSet<String>,
}
/// Recognizes persisted explicit skill prompts without exposing their fragment implementation.
pub fn is_skill_prompt_fragment(text: &str) -> bool {
<fragments::SkillInstructions as codex_extension_api::ContextualUserFragment>::matches_text(
codex-rs/ext/skills/src/loader/discovery.rs
+6
View File
@@ -38,6 +38,7 @@ pub(super) struct SkillDiscovery {
pub plugin_roots: HashSet<PathUri>,
pub namespace_roots: HashSet<PathUri>,
pub warnings: Vec<String>,
pub error: Option<String>,
}
pub(super) struct DiscoveredSkill {
@@ -61,6 +62,7 @@ pub(super) async fn discover_skills(
plugin_roots: HashSet::new(),
namespace_roots: HashSet::new(),
warnings: Vec::new(),
error: None,
};
let walk = match file_system
.walk(
@@ -88,6 +90,9 @@ pub(super) async fn discover_skills(
Err(error) if error.kind() == io::ErrorKind::NotFound => return empty_discovery(),
Err(error) => {
let mut discovery = empty_discovery();
if error.kind() == io::ErrorKind::Unsupported {
discovery.error = Some(error.to_string());
}
discovery
.warnings
.push(format!("failed to walk skills root {root}: {error:#}"));
@@ -173,6 +178,7 @@ pub(super) async fn discover_skills(
plugin_roots,
namespace_roots: HashSet::from([root.clone()]),
warnings,
error: None,
}
}
codex-rs/ext/skills/src/loader/host.rs
+11
View File
@@ -167,6 +167,7 @@ async fn load_skills_under_root(
plugin_roots,
mut namespace_roots,
warnings,
error,
} = discover_skills(
file_system,
&PathUri::from_abs_path(root),
@@ -180,6 +181,16 @@ async fn load_skills_under_root(
for warning in warnings {
error!("{warning}");
}
if let Some(message) = error {
return (
Vec::new(),
Arc::default(),
vec![SkillError {
path: root.clone(),
message,
}],
);
}
if skills.is_empty() {
return (Vec::new(), Arc::default(), Vec::new());
}
codex-rs/ext/skills/src/loader/mod.rs
+1
View File
@@ -17,6 +17,7 @@ pub(crate) use host_merge::load_and_merge_host_skill_roots;
pub(crate) use host_merge::load_and_merge_host_skill_roots_with_request_snapshots;
pub(crate) const MAX_CONCURRENT_ROOT_SCANS: usize = 8;
pub(crate) use discovery::MAX_CONCURRENT_SKILL_LOADS;
pub(super) const SKILLS_FILENAME: &str = "SKILL.md";
pub(super) const SKILLS_METADATA_DIR: &str = "agents";
pub(super) const SKILLS_METADATA_FILENAME: &str = "openai.yaml";
codex-rs/ext/skills/src/preparation.rs
+1 -3
View File
@@ -111,9 +111,7 @@ async fn discover_host_catalog(
input: &StepPreparationInput<'_>,
config: &SkillsExtensionConfig,
) -> Option<SkillCatalog> {
let host_snapshot = input
.turn_store
.get::<HostSkillsSnapshot>()
let host_snapshot = HostSkillsSnapshot::for_step(input.step_store, input.turn_store)
.filter(|_| providers.has_host_provider())?;
if !config.include_instructions && !config.shadow_selection_enabled {
return Some(SkillCatalog::default());
codex-rs/ext/skills/src/render_dedup.rs
+23 -1
View File
@@ -5,6 +5,8 @@ use std::borrow::Cow;
use std::collections::BTreeMap;
use std::collections::HashSet;
use codex_protocol::protocol::SkillScope;
use super::AvailableSkillsRender;
use super::CatalogLines;
use super::SkillCatalogRenderPolicy;
@@ -17,7 +19,9 @@ use super::render_catalog;
use crate::aliases::AliasPlan;
use crate::catalog::SkillCatalog;
use crate::catalog::SkillCatalogEntry;
use crate::catalog::SkillSourceKind;
use crate::catalog_delta::CatalogSkill;
use crate::catalog_delta::GrantStatus;
use crate::catalog_delta::SkillEntrySnapshot;
use crate::catalog_prompt::SkillPromptKind;
@@ -82,6 +86,8 @@ impl<'a> PreparedSkillCatalog<'a> {
pub(crate) fn entries_for_render(
&self,
rendered: &AvailableSkillsRender,
grants: Option<&crate::TurnSkillFilesystemGrants>,
primary_environment: Option<&str>,
) -> BTreeMap<String, CatalogSkill> {
let lines = if rendered.skill_root_lines.is_empty() {
self.unaliased()
@@ -118,10 +124,26 @@ impl<'a> PreparedSkillCatalog<'a> {
.and_then(|plan| plan.root_for_locator(locator))
.cloned();
let shown = shown.as_ref().is_none_or(|ids| ids.contains(&id));
let environment = match &entry.authority.kind {
SkillSourceKind::Executor => entry.main_prompt.environment_id(),
SkillSourceKind::Host if entry.prompt_scope() == Some(SkillScope::Repo) => {
primary_environment
}
SkillSourceKind::Host | SkillSourceKind::Cloud | SkillSourceKind::Custom(_) => {
None
}
};
let granted = environment.is_some_and(|id| {
grants.is_some_and(|grants| grants.environment_ids.contains(id))
});
(
id,
CatalogSkill {
snapshot: SkillEntrySnapshot { fingerprint, shown },
snapshot: SkillEntrySnapshot {
fingerprint,
shown,
grant: granted.then_some(GrantStatus::Active),
},
line: line.render_full(),
root,
},
codex-rs/ext/skills/src/render_tests.rs
+3 -1
View File
@@ -65,7 +65,9 @@ fn edits_beyond_the_catalog_description_cap_keep_the_same_fingerprint() {
)
.unwrap();
prepared
.entries_for_render(&rendered)
.entries_for_render(
&rendered, /*grants*/ None, /*primary_environment*/ None,
)
.into_values()
.next()
.unwrap()
codex-rs/ext/skills/src/world_state_catalogs.rs
+15 -1
View File
@@ -251,13 +251,26 @@ impl<'a> CatalogRenderingContext<'a> {
SkillCatalogRenderPolicy::CORE_COMPATIBLE
.with_omitted_descriptions(self.step_state.config.omit_descriptions),
);
let grants = self
.input
.step_store
.get::<crate::TurnSkillFilesystemGrants>();
let primary_environment = self
.input
.environments
.first()
.map(|selection| selection.environment_id.as_str());
for (catalog, rendered) in [
(&executor, &mut rendered.executor),
(&cloud, &mut rendered.cloud),
(&host, &mut rendered.host),
] {
if let Some(rendered) = rendered {
rendered.entries = catalog.entries_for_render(rendered);
rendered.entries = catalog.entries_for_render(
rendered,
grants.as_deref(),
primary_environment,
);
}
}
}
@@ -438,6 +451,7 @@ impl<'a> CatalogRenderingContext<'a> {
CatalogDelta {
rendered,
include_usage: self.include_usage,
tracks_grants: kind != CatalogKind::Cloud,
allocation: (kind == CatalogKind::Cloud).then_some(allocation),
},
self.input
codex-rs/ext/skills/tests/executor_file_system_authority.rs
+12
View File
@@ -1,3 +1,4 @@
use codex_skills::ImplicitSkillLookup;
use std::collections::HashMap;
use std::io;
use std::sync::Arc;
@@ -88,6 +89,9 @@ impl SyntheticFileSystem {
if path == &self.alias_root {
return Ok(self.canonical_root.clone());
}
if path == &self.path("skill/scripts")? {
return self.path("scripts-target");
}
self.metadata(path)?;
Ok(path.clone())
}
@@ -305,6 +309,14 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() {
let skill = outcome.skills[0].clone();
assert_eq!(skill.name, "synthetic");
assert_eq!(
outcome.implicit_skill_for_scripts_dir(&canonical_root.join("scripts-target")),
Some(&skill),
);
assert_eq!(
outcome.implicit_skill_for_doc_path(&canonical_root.join("skill/SKILL.md")),
Some(&skill),
);
assert_eq!(
skill.path_to_skills_md,
PathUri::from_abs_path(&canonical_root.join("skill/SKILL.md"))
codex-rs/ext/skills/tests/skills_extension/catalog_delta_tests.rs
+184 -23
View File
@@ -86,25 +86,186 @@ async fn skill_sources_spend_the_same_remaining_budget() -> TestResult {
Ok(())
}
#[tokio::test]
async fn grant_expiry_withdraws_only_missing_skills_when_metadata_budget_is_exhausted() -> TestResult
{
let enabled = TestConfig {
catalog_deltas_enabled: true,
..default_config()
};
let survivor = [("permanent", "Always available.")];
let original = [("unrelated", "Removed independently.")];
for grant_when_removed in [None, Some("granting-turn")] {
let mut previous = serde_json::Map::new();
let baseline = render_catalogs_for_step(
&original,
&enabled,
&mut previous,
/*granting_turn*/ None,
/*context_window*/ 60_000,
)
.await?;
assert!(baseline.contains("- executor-unrelated:"), "{baseline}");
let both = [
original[0],
survivor[0],
("temporary", "Turn only."),
("env-b/independent", "Available without the grant on env-1."),
];
let granted = render_catalogs_for_step(
&both,
&enabled,
&mut previous,
Some("granting-turn"),
/*context_window*/ 60_000,
)
.await?;
assert!(granted.contains("- executor-permanent:"), "{granted}");
assert!(granted.contains("- executor-independent:"), "{granted}");
assert!(granted.contains("- executor-temporary:"), "{granted}");
let expired = render_catalogs_for_step(
&survivor,
&enabled,
&mut previous,
grant_when_removed,
/*context_window*/ 1,
)
.await?;
assert_eq!(
expired,
format!("\n## Selected-environment skills update\n{REVALIDATION}")
);
assert_eq!(
render_catalogs_for_step(
&survivor,
&enabled,
&mut previous,
/*granting_turn*/ None,
/*context_window*/ 1
)
.await?,
""
);
let restored = [survivor[0], ("temporary", "Turn only.")];
for expected in [RESTORATION, ""] {
assert_eq!(
render_catalogs_for_step(
&restored,
&enabled,
&mut previous,
Some("reapproved"),
/*context_window*/ 1,
)
.await?,
expected
);
}
assert_eq!(
render_catalogs_for_step(
&survivor,
&enabled,
&mut previous,
/*granting_turn*/ None,
/*context_window*/ 1,
)
.await?,
format!("\n## Selected-environment skills update\n{REVALIDATION}")
);
assert_eq!(
render_catalogs_for_step(
&[],
&enabled,
&mut previous,
/*granting_turn*/ None,
/*context_window*/ 1
)
.await?,
""
);
}
Ok(())
}
const RESTORATION: &str = "\n## Selected-environment skills update\nSome previously unavailable skills are available again. Verify current access before relying on a skill from this source.\n";
const REVALIDATION: &str = "Some previously listed skills are no longer available. Verify current access before relying on a skill from this source.\n";
#[tokio::test]
async fn legacy_filesystem_catalog_revalidates_missing_rows_once() -> TestResult {
let config = TestConfig {
catalog_deltas_enabled: true,
..default_config()
};
let mut previous = serde_json::Map::new();
let survivor = [("stay", "Keep this skill.")];
let both = [survivor[0], ("gone", "Temporarily available.")];
render_catalogs_for_step(
&both,
&config,
&mut previous,
/*granting_turn*/ None,
/*context_window*/ 60_000,
)
.await?;
for snapshot in previous.values_mut() {
snapshot.as_object_mut().unwrap().remove("tracksGrants");
}
assert_eq!(
render_catalogs(&survivor, &config, &mut previous).await?,
format!(
"\n## Selected-environment skills update\n{REVALIDATION}\n## Host skills update\n{REVALIDATION}"
)
);
assert_eq!(
render_catalogs(&survivor, &config, &mut previous).await?,
""
);
assert_eq!(render_catalogs(&[], &config, &mut previous).await?, "");
Ok(())
}
async fn render_catalogs(
skills: &[(&str, &str)],
config: &TestConfig,
previous: &mut serde_json::Map<String, serde_json::Value>,
) -> Result<String, Box<dyn std::error::Error>> {
let provider = |kind: SkillSourceKind| {
let entries = skills
.iter()
.map(|(name, description)| {
let name = format!("{kind}-{name}");
let path = format!("/skills/{name}/SKILL.md");
let mut entry =
test_entry(kind.clone(), "skills", &format!("{kind}/{name}"), &path)
.with_display_path(path);
entry.description = description.to_string();
entry
})
.collect();
Arc::new(StaticSkillProvider {
render_catalogs_for_step(
skills, config, previous, /*granting_turn*/ None, /*context_window*/ 5_000,
)
.await
}
async fn render_catalogs_for_step(
skills: &[(&str, &str)],
config: &TestConfig,
previous: &mut serde_json::Map<String, serde_json::Value>,
granting_turn: Option<&str>,
context_window: i64,
) -> Result<String, Box<dyn std::error::Error>> {
let turn_id = granting_turn.unwrap_or("turn");
let step_store = ExtensionData::new("step");
if granting_turn.is_some() {
step_store.insert(codex_skills_extension::TurnSkillFilesystemGrants {
environment_ids: ["env-1".to_string()].into(),
});
}
let provider = |kind: SkillSourceKind| -> Result<_, codex_utils_path_uri::PathUriParseError> {
let mut entries = Vec::new();
for &(name, description) in skills {
let (environment, name) = name.split_once('/').unwrap_or(("env-1", name));
let name = format!("{kind}-{name}");
let path = format!("/skills/{name}/SKILL.md");
let mut entry = test_entry(kind.clone(), "skills", &format!("{kind}/{name}"), &path);
if kind == SkillSourceKind::Executor {
entry.main_prompt = SkillResourceId::environment(
&path,
environment,
PathUri::parse(&format!("file://{path}"))?,
);
}
entry.description = description.to_string();
entries.push(entry.with_display_path(path));
}
Ok(Arc::new(StaticSkillProvider {
catalog: SkillCatalog {
entries,
..Default::default()
@@ -112,12 +273,12 @@ async fn render_catalogs(
read_requests: Arc::new(Mutex::new(Vec::new())),
list_calls: None,
fail_first_list: false,
})
}))
};
let providers = SkillProviders::new()
.with_executor_provider(provider(SkillSourceKind::Executor))
.with_cloud_provider(provider(SkillSourceKind::Cloud))
.with_host_provider(provider(SkillSourceKind::Host));
.with_executor_provider(provider(SkillSourceKind::Executor)?)
.with_cloud_provider(provider(SkillSourceKind::Cloud)?)
.with_host_provider(provider(SkillSourceKind::Host)?);
let mut builder = ExtensionRegistryBuilder::new();
install_with_providers(&mut builder, providers, skills_extension_config);
let registry = builder.build();
@@ -135,7 +296,7 @@ async fn render_catalogs(
thread_store: &thread_store,
})
.await;
start_registered_turn(&registry, &session_store, &thread_store, "turn").await;
start_registered_turn(&registry, &session_store, &thread_store, turn_id).await;
let roots = [SelectedCapabilityRoot {
id: "skills".to_string(),
location: CapabilityRootLocation::Environment {
@@ -143,7 +304,7 @@ async fn render_catalogs(
path: PathUri::parse("file:///skills")?,
},
}];
let turn_store = ExtensionData::new("turn");
let turn_store = ExtensionData::new(turn_id);
turn_store.insert(HostSkillsSnapshot::new(Arc::new(
SkillLoadOutcome::default(),
)));
@@ -152,14 +313,14 @@ async fn render_catalogs(
&roots,
WorldStateContributionInput {
previous_world_state: Some(previous),
step_store: &ExtensionData::new("step"),
step_store: &step_store,
model_info: &ModelInfo {
context_window: Some(5_000),
context_window: Some(context_window),
include_skills_usage_instructions: true,
..catalog_model_info()
},
thread_id: codex_protocol::ThreadId::new(),
turn_id: "turn",
turn_id,
environments: &[],
extension_metrics: None,
session_store: &session_store,