diff --git a/.codex/skills/remote-tests/SKILL.md b/.codex/skills/remote-tests/SKILL.md index ee35fc2b218..ffacb44db27 100644 --- a/.codex/skills/remote-tests/SKILL.md +++ b/.codex/skills/remote-tests/SKILL.md @@ -1,6 +1,26 @@ --- name: remote-tests description: How to run tests using remote executor. +commands: + - name: build-remote-env + source: repo + example_argv: ["./scripts/test-remote-env.sh"] + purpose: Build and initialize the Docker container used by remote executor tests. + - name: list-devboxes + source: external + example_argv: ["applied_devbox", "ls"] + purpose: List available devboxes before selecting one with codex in the name. + - name: connect-devbox + source: external + example_argv: ["ssh", ""] + purpose: Connect to the selected Linux devbox. +workflow_defaults: + - name: remote_env + value: CODEX_TEST_REMOTE_ENV + description: Set when integration tests should use a remote executor. + - name: remote_checkout + value: ~/code/codex + description: Reuse the devbox checkout and keep SHA and modified files in sync. --- Some codex integration tests support a running against a remote executor. diff --git a/.codex/skills/test-tui/SKILL.md b/.codex/skills/test-tui/SKILL.md index e58e67730ef..395b294eea2 100644 --- a/.codex/skills/test-tui/SKILL.md +++ b/.codex/skills/test-tui/SKILL.md @@ -1,6 +1,18 @@ --- name: test-tui description: Guide for testing Codex TUI interactively +commands: + - name: start-tui + source: repo + example_argv: ["just", "codex", "-c", "log_dir="] + purpose: Start the TUI interactively from the repo recipe with logs directed to a temp directory. +workflow_defaults: + - name: rust_log + value: RUST_LOG=trace + description: Always set trace logging when starting the TUI for interactive testing. + - name: input_delivery + value: text_then_enter + description: Send text first, then Enter as a separate write when driving the TUI programmatically. --- You can start and use Codex TUI to verify changes. diff --git a/code-rs/core/src/skills/loader.rs b/code-rs/core/src/skills/loader.rs index c1e3ec90455..792d1309ee4 100644 --- a/code-rs/core/src/skills/loader.rs +++ b/code-rs/core/src/skills/loader.rs @@ -8,6 +8,7 @@ use crate::skills::model::SkillCommandPolicy; use crate::skills::model::SkillCommandPolicyAction; use crate::skills::model::SkillCommandPolicyPreferred; use crate::skills::model::SkillCommandPolicyPreferredKind; +use crate::skills::model::SkillCommandSource; use crate::skills::model::SkillError; use crate::skills::model::SkillLoadOutcome; use crate::skills::model::SkillMetadata; @@ -71,12 +72,24 @@ enum SkillFrontmatterResourceKind { #[derive(Debug, Deserialize)] struct SkillFrontmatterCommand { name: String, - resource_path: PathBuf, + #[serde(default)] + source: SkillFrontmatterCommandSource, + #[serde(default)] + resource_path: Option, #[serde(default)] example_argv: Vec, purpose: String, } +#[derive(Debug, Deserialize, Default)] +#[serde(rename_all = "snake_case")] +enum SkillFrontmatterCommandSource { + #[default] + Skill, + Repo, + External, +} + #[derive(Debug, Deserialize)] struct SkillFrontmatterWorkflowDefault { name: String, @@ -484,7 +497,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result, skill_dir: &Path, + scope: SkillScope, declared_resources: &[SkillResource], ) -> Result, SkillParseError> { if commands.len() > MAX_STRUCTURED_ITEMS_PER_SKILL { @@ -566,30 +580,19 @@ fn parse_skill_commands( validate_field(&name, MAX_STRUCTURED_ID_LEN, "commands.name")?; let purpose = sanitize_single_line(&command.purpose); validate_field(&purpose, MAX_STRUCTURED_PURPOSE_LEN, "commands.purpose")?; - let resolved_resource_path = resolve_skill_relative_path(skill_dir, command.resource_path.clone()); - - let declared_resource = declared_resources - .iter() - .find(|r| r.path == resolved_resource_path); - let Some(declared_resource) = declared_resource else { - return Err(SkillParseError::InvalidField { - field: "commands.resource_path", - reason: format!( - "command '{}' references resource_path '{}' which is not declared in the resources section", - name, command.resource_path.display() - ), - }); + let source = match command.source { + SkillFrontmatterCommandSource::Skill => SkillCommandSource::Skill, + SkillFrontmatterCommandSource::Repo => SkillCommandSource::Repo, + SkillFrontmatterCommandSource::External => SkillCommandSource::External, }; - - if !declared_resource.path.is_file() { - return Err(SkillParseError::InvalidField { - field: "commands.resource_path", - reason: format!( - "command '{}' references resource_path '{}' which does not exist as a file", - name, command.resource_path.display() - ), - }); - } + let resource_path = parse_command_resource_path( + &name, + source, + scope, + command.resource_path, + skill_dir, + declared_resources, + )?; let example_argv = if command.example_argv.is_empty() { Vec::new() @@ -599,7 +602,8 @@ fn parse_skill_commands( parsed.push(SkillCommand { name, - resource_path: resolved_resource_path, + source, + resource_path, example_argv, purpose, }); @@ -607,6 +611,77 @@ fn parse_skill_commands( Ok(parsed) } +fn parse_command_resource_path( + name: &str, + source: SkillCommandSource, + scope: SkillScope, + resource_path: Option, + skill_dir: &Path, + declared_resources: &[SkillResource], +) -> Result, SkillParseError> { + match source { + SkillCommandSource::Skill => { + let Some(resource_path) = resource_path else { + return Err(SkillParseError::MissingField("commands.resource_path")); + }; + let resolved_resource_path = resolve_skill_relative_path(skill_dir, resource_path.clone()); + + let declared_resource = declared_resources + .iter() + .find(|r| r.path == resolved_resource_path); + let Some(declared_resource) = declared_resource else { + return Err(SkillParseError::InvalidField { + field: "commands.resource_path", + reason: format!( + "command '{}' references resource_path '{}' which is not declared in the resources section", + name, resource_path.display() + ), + }); + }; + + if !declared_resource.path.is_file() { + return Err(SkillParseError::InvalidField { + field: "commands.resource_path", + reason: format!( + "command '{}' references resource_path '{}' which does not exist as a file", + name, resource_path.display() + ), + }); + } + + Ok(Some(resolved_resource_path)) + } + SkillCommandSource::Repo => { + if scope != SkillScope::Repo { + return Err(SkillParseError::InvalidField { + field: "commands.source", + reason: format!("command '{name}' uses source repo outside a repo skill"), + }); + } + if resource_path.is_some() { + return Err(SkillParseError::InvalidField { + field: "commands.resource_path", + reason: format!( + "command '{name}' with source repo or external must not declare resource_path" + ), + }); + } + Ok(None) + } + SkillCommandSource::External => { + if resource_path.is_some() { + return Err(SkillParseError::InvalidField { + field: "commands.resource_path", + reason: format!( + "command '{name}' with source repo or external must not declare resource_path" + ), + }); + } + Ok(None) + } + } +} + fn parse_optional_text( value: Option, max_len: usize, @@ -1178,6 +1253,44 @@ workflow_defaults: ); } + #[test] + fn repo_commands_require_repo_scope() { + let skills_root = tempfile::tempdir().expect("tempdir"); + let skill_dir = skills_root.path().join("bad"); + fs::create_dir_all(&skill_dir).expect("create skill dir"); + fs::write( + skill_dir.join(SKILLS_FILENAME), + r#"--- +name: bad +description: Bad command source +commands: + - name: bad-repo-command + source: repo + example_argv: ["./scripts/helper.sh"] + purpose: Uses repo command source outside repo scope. +--- + +# bad +"#, + ) + .expect("write skill file"); + + let outcome = load_skills_from_roots(vec![SkillRoot { + path: skills_root.path().to_path_buf(), + scope: SkillScope::User, + }]); + + assert!(outcome.skills.is_empty()); + assert_eq!(outcome.errors.len(), 1); + assert!( + outcome.errors[0] + .message + .contains("uses source repo outside a repo skill"), + "unexpected error: {}", + outcome.errors[0].message + ); + } + #[test] fn loads_structured_skill_workflow_fields_from_frontmatter() { let skills_root = tempfile::tempdir().expect("tempdir"); @@ -1213,9 +1326,10 @@ workflow_defaults: assert_eq!(skill.commands.len(), 2); assert_eq!(skill.commands[0].name, "create-plan"); + assert_eq!(skill.commands[0].source, SkillCommandSource::Skill); assert_eq!( - skill.commands[0].resource_path, - normalized(&skill_path).parent().unwrap().join("scripts/create_plan.py") + skill.commands[0].resource_path.as_deref(), + Some(normalized(&skill_path).parent().unwrap().join("scripts/create_plan.py").as_path()) ); assert_eq!( skill.commands[0].example_argv, @@ -1224,9 +1338,10 @@ workflow_defaults: assert_eq!(skill.commands[0].purpose, "Create a saved plan."); assert_eq!(skill.commands[1].name, "list-plans"); + assert_eq!(skill.commands[1].source, SkillCommandSource::Skill); assert_eq!( - skill.commands[1].resource_path, - normalized(&skill_path).parent().unwrap().join("scripts/list_plans.py") + skill.commands[1].resource_path.as_deref(), + Some(normalized(&skill_path).parent().unwrap().join("scripts/list_plans.py").as_path()) ); assert_eq!( skill.commands[1].example_argv, @@ -1324,6 +1439,91 @@ commands: ); } + #[test] + fn repo_and_external_commands_do_not_require_skill_resources() { + let skills_root = tempfile::tempdir().expect("tempdir"); + let skill_dir = skills_root.path().join("remote-tests"); + fs::create_dir_all(&skill_dir).expect("create skill dir"); + fs::write( + skill_dir.join(SKILLS_FILENAME), + r#"--- +name: remote-tests +description: Run remote tests +commands: + - name: build-remote-env + source: repo + example_argv: ["./scripts/test-remote-env.sh"] + purpose: Build the remote test executor environment. + - name: list-devboxes + source: external + example_argv: ["applied_devbox", "ls"] + purpose: List available devboxes. +--- + +# remote-tests +"#, + ) + .expect("write skill file"); + + let outcome = load_skills_from_roots(vec![SkillRoot { + path: skills_root.path().to_path_buf(), + scope: SkillScope::Repo, + }]); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + let skill = outcome.skills.first().expect("skill"); + assert_eq!(skill.commands.len(), 2); + assert_eq!(skill.commands[0].source, SkillCommandSource::Repo); + assert_eq!(skill.commands[0].resource_path, None); + assert_eq!(skill.commands[0].example_argv, ["./scripts/test-remote-env.sh"]); + assert_eq!(skill.commands[1].source, SkillCommandSource::External); + assert_eq!(skill.commands[1].resource_path, None); + assert_eq!(skill.commands[1].example_argv, ["applied_devbox", "ls"]); + } + + #[test] + fn repo_and_external_commands_must_not_declare_resource_path() { + let skills_root = tempfile::tempdir().expect("tempdir"); + let skill_dir = skills_root.path().join("bad"); + fs::create_dir_all(&skill_dir).expect("create skill dir"); + fs::write( + skill_dir.join(SKILLS_FILENAME), + r#"--- +name: bad +description: Bad command source +commands: + - name: bad-repo-command + source: repo + resource_path: scripts/helper.sh + example_argv: ["./scripts/helper.sh"] + purpose: Mixes command source fields. +--- + +# bad +"#, + ) + .expect("write skill file"); + + let outcome = load_skills_from_roots(vec![SkillRoot { + path: skills_root.path().to_path_buf(), + scope: SkillScope::Repo, + }]); + + assert!(outcome.skills.is_empty()); + assert_eq!(outcome.errors.len(), 1); + assert!( + outcome.errors[0] + .message + .contains("must not declare resource_path"), + "unexpected error: {}", + outcome.errors[0].message + ); + } + #[test] fn invalid_command_policy_reports_skill_error() { let skills_root = tempfile::tempdir().expect("tempdir"); @@ -1938,7 +2138,8 @@ Plan skill body assert_eq!(skill.commands.len(), 1); assert_eq!(skill.commands[0].name, "create-plan"); - assert_eq!(skill.commands[0].resource_path, normalized(&skill_dir.join("create_plan.py"))); + assert_eq!(skill.commands[0].source, SkillCommandSource::Skill); + assert_eq!(skill.commands[0].resource_path.as_deref(), Some(normalized(&skill_dir.join("create_plan.py")).as_path())); assert_eq!(skill.commands[0].example_argv, vec!["python", "create_plan.py", "--name"]); assert_eq!(skill.commands[0].purpose, "Create a plan file"); } diff --git a/code-rs/core/src/skills/model.rs b/code-rs/core/src/skills/model.rs index 477047818fc..a1d8d4d4bb1 100644 --- a/code-rs/core/src/skills/model.rs +++ b/code-rs/core/src/skills/model.rs @@ -49,11 +49,19 @@ pub enum SkillResourceKind { #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillCommand { pub name: String, - pub resource_path: PathBuf, + pub source: SkillCommandSource, + pub resource_path: Option, pub example_argv: Vec, pub purpose: String, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillCommandSource { + Skill, + Repo, + External, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillWorkflowDefault { pub name: String, diff --git a/code-rs/core/src/skills/render.rs b/code-rs/core/src/skills/render.rs index 53efdb4afbc..93108d6bf11 100644 --- a/code-rs/core/src/skills/render.rs +++ b/code-rs/core/src/skills/render.rs @@ -1,5 +1,6 @@ use crate::skills::model::SkillMetadata; use crate::skills::model::SkillResourceKind; +use crate::skills::model::SkillCommandSource; use crate::skills::command_policy::render_command_policy_summary; pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { @@ -90,13 +91,30 @@ fn render_structured_skill_guidance(skill: &SkillMetadata) -> Vec { } else { command.example_argv.join(" ") }; - let line = format!( - " - command `{}`: run `{}` (helper script: {}); purpose: {}", - command.name, - example, - command.resource_path.to_string_lossy().replace('\\', "/"), - command.purpose - ); + let line = match command.source { + SkillCommandSource::Skill => { + let helper_path = command + .resource_path + .as_ref() + .map(|path| path.to_string_lossy().replace('\\', "/")) + .unwrap_or_else(|| "".to_string()); + format!( + " - command `{}`: run `{}` (helper script: {}); purpose: {}", + command.name, + example, + helper_path, + command.purpose + ) + } + SkillCommandSource::Repo => format!( + " - command `{}`: run `{}` (repo command); purpose: {}", + command.name, example, command.purpose + ), + SkillCommandSource::External => format!( + " - command `{}`: run `{}` (external command); purpose: {}", + command.name, example, command.purpose + ), + }; lines.push(line); } @@ -120,6 +138,7 @@ mod tests { use crate::skills::model::SkillCommandPolicyAction; use crate::skills::model::SkillCommandPolicyPreferred; use crate::skills::model::SkillCommandPolicyPreferredKind; + use crate::skills::model::SkillCommandSource; use crate::skills::model::SkillPolicy; use crate::skills::model::SkillResource; use crate::skills::model::SkillResourceKind; @@ -242,10 +261,25 @@ mod tests { skill.commands = vec![ SkillCommand { name: "create-plan".to_string(), - resource_path: PathBuf::from("/tmp/plan/scripts/create_plan.py"), + source: SkillCommandSource::Skill, + resource_path: Some(PathBuf::from("/tmp/plan/scripts/create_plan.py")), example_argv: vec!["python".to_string(), "scripts/create_plan.py".to_string(), "--name".to_string()], purpose: "Create and save a new plan".to_string(), }, + SkillCommand { + name: "remote-env".to_string(), + source: SkillCommandSource::Repo, + resource_path: None, + example_argv: vec!["./scripts/test-remote-env.sh".to_string()], + purpose: "Build remote executor environment".to_string(), + }, + SkillCommand { + name: "list-devboxes".to_string(), + source: SkillCommandSource::External, + resource_path: None, + example_argv: vec!["applied_devbox".to_string(), "ls".to_string()], + purpose: "List available devboxes".to_string(), + }, ]; skill.workflow_defaults = vec![SkillWorkflowDefault { name: "save_location".to_string(), @@ -257,6 +291,8 @@ mod tests { assert!(rendered.contains("resource: /tmp/plan/scripts/create_plan.py (script); description: Helper to create a plan")); assert!(rendered.contains("command `create-plan`: run `python scripts/create_plan.py --name` (helper script: /tmp/plan/scripts/create_plan.py); purpose: Create and save a new plan")); + assert!(rendered.contains("command `remote-env`: run `./scripts/test-remote-env.sh` (repo command); purpose: Build remote executor environment")); + assert!(rendered.contains("command `list-devboxes`: run `applied_devbox ls` (external command); purpose: List available devboxes")); assert!(rendered.contains("workflow default `save_location`: $CODEX_HOME/plans; description: Keep plans outside repos")); } @@ -270,7 +306,8 @@ mod tests { }]; skill.commands = vec![SkillCommand { name: "create-plan".to_string(), - resource_path: PathBuf::from("/tmp/plan/scripts/create_plan.py"), + source: SkillCommandSource::Skill, + resource_path: Some(PathBuf::from("/tmp/plan/scripts/create_plan.py")), example_argv: vec!["python".to_string(), "scripts/create_plan.py".to_string()], purpose: "Create and save a new plan".to_string(), }];