diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index a54147e9e0..3f03e9989d 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -1,5 +1,7 @@ //! Multi-turn Astra scenarios snapshot the model-visible request history of shipped features. +use std::collections::HashMap; +use std::collections::HashSet; use std::fs; use std::path::Path; use std::path::PathBuf; @@ -14,21 +16,31 @@ use codex_config::ConfigLayerStack; use codex_config::types::McpServerConfig; use codex_context_fragments::AnsweredQuestion; use codex_context_fragments::ContextualUserFragment; +use codex_core::StartThreadOptions; use codex_core::TurnInputRequest; use codex_core::config::Config; +use codex_exec_server::EnvironmentManager; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; +use codex_extension_api::ExtensionDataInit; use codex_extension_api::ExtensionRegistry; use codex_extension_api::ExtensionRegistryBuilder; use codex_features::Feature; use codex_login::CodexAuth; use codex_models_manager::bundled_models_response; +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::items::AgentMessageDelivery; use codex_protocol::items::TurnItem; use codex_protocol::models::ImageReference; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::user_input::UserInput; +use codex_skills_extension::ExecutorSkillProvider; +use codex_skills_extension::SkillProviders; use codex_skills_extension::SkillsExtensionConfig; use codex_skills_extension::install; +use codex_skills_extension::install_with_providers; +use codex_utils_path_uri::PathUri; use core_test_support::context_snapshot; use core_test_support::context_snapshot::ContextSnapshotOptions; use core_test_support::context_snapshot::SnapshotEntry; @@ -409,6 +421,112 @@ async fn astra_kickoff_with_skills_plugins_and_remote_compaction() -> Result<()> Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn astra_omits_disabled_executor_skills_from_model_context() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let skill_files = TempDir::new()?; + let skill_root = fs::canonicalize(skill_files.path())?.join("skills"); + let disabled = write_skill( + &skill_root.join("retired-helper"), + "retired-helper", + "Use the retired workflow", + "Follow the retired workflow.", + )?; + write_skill( + &skill_root.join("active-helper"), + "active-helper", + "Use the current workflow", + "Follow the current workflow.", + )?; + let provider = ExecutorSkillProvider::new_with_restriction_product( + Arc::new(EnvironmentManager::default_for_tests()), + /*restriction_product*/ None, + ) + .with_disabled_skill_paths(HashMap::from([( + LOCAL_ENVIRONMENT_ID.to_string(), + HashSet::from([PathUri::from_host_native_path(disabled)?]), + )])); + let mut extensions = ExtensionRegistryBuilder::::new(); + install_with_providers( + &mut extensions, + SkillProviders::new().with_executor_provider(Arc::new(provider)), + |config: &Config| SkillsExtensionConfig { + include_instructions: config.include_skill_instructions, + max_context_tokens: config.skill_max_context_tokens, + bundled_skills_enabled: false, + orchestrator_skills_enabled: false, + shadow_selection_enabled: false, + }, + ); + let mock = mount_sse_sequence( + &server, + vec![sse(vec![ + ev_assistant_message("skills", "The active-helper skill is available."), + ev_completed("skills-response"), + ])], + ) + .await; + let test = test_codex() + .with_model("gpt-6-astra") + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_extensions(Arc::new(extensions.build())) + .with_config(configure_scenario_catalog) + .build(&server) + .await?; + let skill_root = PathUri::from_host_native_path(skill_root)?; + let root_locator = format!( + "skill://workspace-skills/{}", + skill_root + .inferred_native_path_string() + .replace('\\', "/") + .trim_start_matches('/') + ); + let mut thread_extension_init = ExtensionDataInit::new(); + thread_extension_init.insert(vec![SelectedCapabilityRoot { + id: "workspace-skills".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: LOCAL_ENVIRONMENT_ID.to_string(), + path: skill_root, + }, + }]); + let thread = test + .thread_manager + .start_thread(StartThreadOptions { + thread_extension_init, + ..StartThreadOptions::new(test.config.clone()) + }) + .await? + .thread; + thread + .start_or_steer_turn(TurnInputRequest::user_input(vec![text( + "Which workflow skills are available?", + )])) + .await?; + wait_for_event(&thread, |event| matches!(event, EventMsg::TurnComplete(_))).await; + + let requests = mock.requests(); + assert_eq!(requests.len(), 1); + let mut body = requests[0].body_json(); + let input = body["input"].to_string(); + assert!(input.contains("active-helper")); + assert!(!input.contains("retired-helper")); + // Normalize opaque skill locators before snapshot truncation and hashing. + body["input"] = serde_json::from_str( + &input.replace(&root_locator, "skill://workspace-skills/"), + )?; + insta::assert_snapshot!( + "astra_disabled_executor_skills", + context_snapshot::format_context_snapshot( + "Astra sees the active executor skill while the caller-disabled skill is omitted.", + &[SnapshotEntry::body(&body)], + &ContextSnapshotOptions::default().include_request_settings(), + ) + ); + Ok(()) +} + #[cfg_attr(windows, ignore = "the fixture uses a Unix shell command")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn astra_settings_release_check_with_direct_and_code_mode_tools() -> Result<()> { diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_disabled_executor_skills.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_disabled_executor_skills.snap new file mode 100644 index 0000000000..db98999ae3 --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__astra_disabled_executor_skills.snap @@ -0,0 +1,127 @@ +--- +source: core/tests/suite/scenarios.rs +expression: "context_snapshot::format_context_snapshot(\"Astra sees the active executor skill while the caller-disabled skill is omitted.\",\n&[SnapshotEntry::body(&body)],\n&ContextSnapshotOptions::default().include_request_settings(),)" +--- +Scenario: Astra sees the active executor skill while the caller-disabled skill is omitted. + +## Window 1 +Settings: + include: ["reasoning.encrypted_content"] + model: "gpt-6-astra" + parallel_tool_calls: false + prompt_cache_key: "" + reasoning: {"context":"all_turns","effort":"low"} + store: false + stream: true + text: {"verbosity":"low"} + tool_choice: "auto" +-- request 1 (request) -- +00:additional_tools/developer (3; hash=24819537059703a0): + - namespace/functions; hash=97852c93915dad58 + - custom/exec + - function/wait + - function/request_user_input + - function/request_user_input_async + - namespace/clock: Tools for reading and waiting on time.; hash=f402e8c5e9b5e317 + - function/sleep + - namespace/collaboration: Tools for spawning and managing sub-agents.; hash=43f195d9cc99ee15 + - function/followup_task + - function/interrupt_agent + - function/list_agents + - function/send_message + - function/spawn_agent + - function/wait_agent +01:message/developer: + You are Codex, an agent based on GPT-6. You and the user share one workspace, and your job is to collabo...them until their intended goal is completely handled. [hash=efa50f074efa5803] + + # When to ask the user for permission + + Use your best judgement given task context for when you really need user permission, like a competent co...ork without ending the turn to clarify with the user. [hash=ca7367e658b51388] + + User authorization and preferences persist across turns. Do not request permission again when the user h... any guidelines provided in skills or external files. [hash=911f91c92c72882c] + + + + - Skill naming: If a plugin contributes skills, those skill entries are prefixed with plugin_name: in the Skills list. + - MCP naming: Plugin-provided MCP tools keep standard MCP identifiers such as mcp__server__tool; use tool provenance to tell which plugin they come from. + - Trigger rules: If the user explicitly names a plugin, prefer capabilities associated with that plugin for that turn. + - Relationship to capabilities: Plugins are not invoked directly. Use their underlying skills, MCP tools, and app tools to help solve the task. + - Relevance: Determine what a plugin can help with from explicit user mention or from the plugin-associa..., MCP tools, and apps exposed elsewhere in this turn. [hash=620459a4e7b590f0] + - Missing/blocked: If the user requests a plugin that does not have relevant callable capabilities for t..., say so briefly and continue with the best fallback. [hash=746b0469a8f6cb3e] + +02:message/developer[3]: + [01] + 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: &&, || + + NEVER provide a prefix_rule if your command uses a heredoc or herestring. + + ### Examples + Good examples of prefixes: + - ["npm", "run", "dev"] + - ["gh", "pr", "check"] + - ["cargo", "test"] + + [02] # Collaboration Mode: Default + + You are now in Default mode. Any previous instructions for other modes (e.g. Plan mode) are no longer active. + + Your active mode changes only when new developer instructions with a different `...<...by themselves. Known mode names are Default and Plan. [hash=e75f6b12fc52d812] + + ## request_user_input availability + + Use the `request_user_input` tool only when it is listed in the available tools for this turn. + + In Default mode, strongly prefer making reasonable assumptions and executing the user's request rather than stopping to ask questions. + + Use the `request_user_input` tool only for optional questions where the answer would materially improve the quality of the work. + + If `request_user_input` returns no answers, continue with best judgment instead of asking again or treating the turn as blocked. + + Never use the `request_user_input` tool for permission requests or permission-related escalations. + + If explicit user input is required for another reason before progress can safely continue, do not use th... as a textual assistant message. [hash=8213052bde4a62b2] + [03] + ## Skills + A skill is a set of instructions provided through a `SKILL.md` source. Below is the list of skills that ...locators can be expanded using the skill roots table. [hash=f00d6a4d3e192f06] + ### Skill roots + - `e0` = `skill://workspace-skills/` + Read a skill package directly with `skills.read({"package":""})` to read its `SKILL.md`; root a...ackage is not provided, use `skills.list` to find it. [hash=25bf84ea2123d647] + ### Available skills + - active-helper: Use the current workflow (executor package: e0/active-helper) + +03:message/developer: + You are `/root`, the primary agent in a team of agents collaborating to fulfill the user's goals. + + At the start of your turn, you are the active agent. + You can spawn sub-agents to handle subtasks, and those sub-agents can spawn their own sub-agents. + All agents in the team, including the agents that you can assign tasks to, are equally intelligent and capable, and have access to the same set of tools. + + You can use `spawn_agent` to create a new agent, `followup_task` to give an existing agent a new task an...message to a running agent without triggering a turn. [hash=7d40ff8b58977202] + `send_message` calls may be read by a human, so ensure they are legible. Always put proper spaces between words and/or numbers. + + - All agents use the same current working directory. + - As a result, edits made by one agent are immediately visible to all other agents. + + When calling `wait_agent`, prefer longer waits (minutes) to avoid busy polling. + + There are 4 available concurrency slots, meaning that up to 4 agents can be active at once, including you. + + Full-history forks (`fork_turns` omitted or `"all"`) inherit the parent model and reasoning effort and d...ne"` or a positive integer string. [hash=c7f52e90688299ce] +04:message/developer: + Any earlier instruction enabling proactive multi-agent delegation no longer applies. D...elegation, or parallel agent work. [hash=2b6e037067b4390c] +05:message/user: + + + + + + [hash=911722cc43b4c3b8] + +06:message/user: + Which workflow skills are available? diff --git a/codex-rs/ext/skills/src/provider/executor.rs b/codex-rs/ext/skills/src/provider/executor.rs index 5010614853..c6717e74c9 100644 --- a/codex-rs/ext/skills/src/provider/executor.rs +++ b/codex-rs/ext/skills/src/provider/executor.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; +use std::collections::HashSet; use std::sync::Arc; use codex_exec_server::EnvironmentManager; @@ -34,6 +36,7 @@ use crate::provider::SkillSearchRequest; pub struct ExecutorSkillProvider { environment_manager: Arc, restriction_product: Option, + disabled_skill_paths: HashMap>, } impl ExecutorSkillProvider { @@ -44,8 +47,20 @@ impl ExecutorSkillProvider { Self { environment_manager, restriction_product, + disabled_skill_paths: HashMap::new(), } } + + /// Applies caller-owned disablement to executor catalogs. Paths identify SKILL.md + /// documents in their executor's filesystem; other executors are unaffected. + /// By default, all discovered skills remain enabled. + pub fn with_disabled_skill_paths( + mut self, + disabled_skill_paths: HashMap>, + ) -> Self { + self.disabled_skill_paths = disabled_skill_paths; + self + } } pub(crate) fn attribute_executor_plugins( @@ -113,6 +128,7 @@ impl SkillProvider for ExecutorSkillProvider { path, environment_id, /*instructions*/ None, + &self.disabled_skill_paths, )); } } @@ -223,6 +239,7 @@ impl ExecutorSkillProvider { path, environment_id, Some(skill.instructions), + &self.disabled_skill_paths, )); } } @@ -237,6 +254,7 @@ fn catalog_entry_from_skill( selected_root_path: &PathUri, environment_id: &str, instructions: Option, + disabled_skill_paths: &HashMap>, ) -> SkillCatalogEntry { let handle_prefix = format!("skill://{selected_root_id}/"); let alias_root = format!( @@ -269,7 +287,7 @@ fn catalog_entry_from_skill( skill.path_to_skills_md.clone(), ), }; - let entry = SkillCatalogEntry::new( + let mut entry = SkillCatalogEntry::new( SkillPackageId(package), authority, skill.name.clone(), @@ -281,6 +299,13 @@ fn catalog_entry_from_skill( .with_alias_root(alias_root) .with_dependencies(skill.dependencies.clone()); + if disabled_skill_paths + .get(environment_id) + .is_some_and(|paths| paths.contains(&skill.path_to_skills_md)) + { + entry = entry.disabled(); + } + if skill.allows_implicit_invocation() { entry } else { diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index 5c18550597..1f0f0e6ae6 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -613,7 +613,7 @@ async fn executor_discovery_routes_produce_equivalent_catalog_metadata() { .snapshot(&executor_roots, &Default::default()) .await; let bundled = provider - .list(query(Some(discovery))) + .list(query(Some(discovery.clone()))) .await .expect("list bundled executor skills"); @@ -661,6 +661,7 @@ async fn executor_discovery_routes_produce_equivalent_catalog_metadata() { .find(|entry| entry.name == "catalog:repaired") .expect("repaired skill"); assert_eq!(repaired.description, "Build for AWS: ECS"); + assert!(repaired.prompt_visible); let invalid_metadata = direct .entries .iter() @@ -668,6 +669,45 @@ async fn executor_discovery_routes_produce_equivalent_catalog_metadata() { .expect("invalid metadata skill"); assert_eq!(invalid_metadata.dependencies, None); + assert!(direct.entries.iter().all(|entry| entry.enabled)); + // Match the discovery path, including aliases such as macOS's /var -> /private/var. + let disabled_path = PathUri::from_host_native_path(&repaired_skill).expect("skill URI"); + for environment_id in ["local", "other-executor"] { + let configured = provider.clone().with_disabled_skill_paths(HashMap::from([( + environment_id.to_string(), + std::collections::HashSet::from([disabled_path.clone()]), + )])); + let mut expected = direct.clone(); + if environment_id == "local" { + expected + .entries + .iter_mut() + .find(|entry| entry.name == "catalog:repaired") + .expect("repaired skill") + .enabled = false; + } + for discovery in [None, Some(discovery.clone())] { + let actual = configured + .list(query(discovery)) + .await + .expect("list configured executor skills"); + assert_eq!(comparable_entries(&actual), comparable_entries(&expected)); + assert_eq!(actual.warnings, expected.warnings); + let visible_names = actual + .entries + .iter() + .filter(|entry| entry.enabled && entry.prompt_visible) + .map(|entry| entry.name.as_str()) + .collect::>(); + let expected_visible_names = if environment_id == "local" { + vec!["catalog:invalid-metadata"] + } else { + vec!["catalog:invalid-metadata", "catalog:repaired"] + }; + assert_eq!(visible_names, expected_visible_names); + } + } + std::fs::remove_dir_all(test_root).expect("remove skill directory"); }