diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 62b4d4d9b..acf173c51 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1291,6 +1291,7 @@ dependencies = [ "futures", "http 1.3.1", "image", + "include_dir", "indexmap 2.12.0", "keyring", "landlock", @@ -3366,7 +3367,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.5.10", + "socket2 0.6.1", "system-configuration", "tokio", "tower-service", @@ -3589,6 +3590,25 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e8a5a9a0ff0086c7a148acb942baaabeadf9504d10400b5a05645853729b9cd2" +[[package]] +name = "include_dir" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "923d117408f1e49d914f1a379a309cffe4f18c05cf4e3d12e613a15fc81bd0dd" +dependencies = [ + "include_dir_macros", +] + +[[package]] +name = "include_dir_macros" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cab85a7ed0bd5f0e76d93846e0147172bed2e2d3f859bcc33a8d9699cad1a75" +dependencies = [ + "proc-macro2", + "quote", +] + [[package]] name = "indenter" version = "0.3.3" @@ -5152,7 +5172,7 @@ dependencies = [ "quinn-udp", "rustc-hash", "rustls", - "socket2 0.5.10", + "socket2 0.6.1", "thiserror 2.0.17", "tokio", "tracing", @@ -5189,7 +5209,7 @@ dependencies = [ "cfg_aliases 0.2.1", "libc", "once_cell", - "socket2 0.5.10", + "socket2 0.6.1", "tracing", "windows-sys 0.60.2", ] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 929f74d07..50941771c 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -141,6 +141,7 @@ icu_locale_core = "2.1" icu_provider = { version = "2.1", features = ["sync"] } ignore = "0.4.23" image = { version = "^0.25.9", default-features = false } +include_dir = "0.7.4" indexmap = "2.12.0" insta = "1.44.3" itertools = "0.14.0" diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 19fa0c663..66bc7647b 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -980,6 +980,10 @@ pub struct SkillsListParams { /// When empty, defaults to the current session working directory. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub cwds: Vec, + + /// When true, bypass the skills cache and re-scan skills from disk. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub force_reload: bool, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -996,7 +1000,7 @@ pub struct SkillsListResponse { pub enum SkillScope { User, Repo, - Public, + System, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -1042,7 +1046,7 @@ impl From for SkillScope { match value { CoreSkillScope::User => Self::User, CoreSkillScope::Repo => Self::Repo, - CoreSkillScope::Public => Self::Public, + CoreSkillScope::System => Self::System, } } } @@ -1939,6 +1943,30 @@ mod tests { ); } + #[test] + fn skills_list_params_serialization_uses_force_reload() { + assert_eq!( + serde_json::to_value(SkillsListParams { + cwds: Vec::new(), + force_reload: false, + }) + .unwrap(), + json!({}), + ); + + assert_eq!( + serde_json::to_value(SkillsListParams { + cwds: vec![PathBuf::from("/repo")], + force_reload: true, + }) + .unwrap(), + json!({ + "cwds": ["/repo"], + "forceReload": true, + }), + ); + } + #[test] fn codex_error_info_serializes_http_status_code_in_camel_case() { let value = CodexErrorInfo::ResponseTooManyFailedAttempts { diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 460418c81..2f141c4e1 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -77,7 +77,7 @@ Example (from OpenAI's official VSCode extension): - `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start` and emits `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review. - `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation). - `model/list` — list available models (with reasoning effort options). -- `skills/list` — list skills for one or more `cwd` values. +- `skills/list` — list skills for one or more `cwd` values (optional `forceReload`). - `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; returns an `authorization_url` and later emits `mcpServer/oauthLogin/completed` once the browser flow finishes. - `mcpServerStatus/list` — enumerate configured MCP servers with their tools, resources, resource templates, and auth status; supports cursor+limit pagination. - `feedback/upload` — submit a feedback report (classification + optional reason/logs and conversation_id); returns the tracking thread id. diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 3bc3b064f..2d581e238 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -2640,36 +2640,27 @@ impl CodexMessageProcessor { } async fn skills_list(&self, request_id: RequestId, params: SkillsListParams) { - let SkillsListParams { cwds } = params; + let SkillsListParams { cwds, force_reload } = params; let cwds = if cwds.is_empty() { vec![self.config.cwd.clone()] } else { cwds }; - let data = if self.config.features.enabled(Feature::Skills) { - let skills_manager = self.conversation_manager.skills_manager(); - cwds.into_iter() - .map(|cwd| { - let outcome = skills_manager.skills_for_cwd(&cwd); - let errors = errors_to_info(&outcome.errors); - let skills = skills_to_info(&outcome.skills); - codex_app_server_protocol::SkillsListEntry { - cwd, - skills, - errors, - } - }) - .collect() - } else { - cwds.into_iter() - .map(|cwd| codex_app_server_protocol::SkillsListEntry { + let skills_manager = self.conversation_manager.skills_manager(); + let data = cwds + .into_iter() + .map(|cwd| { + let outcome = skills_manager.skills_for_cwd_with_options(&cwd, force_reload); + let errors = errors_to_info(&outcome.errors); + let skills = skills_to_info(&outcome.skills); + codex_app_server_protocol::SkillsListEntry { cwd, - skills: Vec::new(), - errors: Vec::new(), - }) - .collect() - }; + skills, + errors, + } + }) + .collect(); self.outgoing .send_response(request_id, SkillsListResponse { data }) .await; diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 0ba8e1fd3..2b51b784c 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -43,6 +43,7 @@ env-flags = { workspace = true } eventsource-stream = { workspace = true } futures = { workspace = true } http = { workspace = true } +include_dir = { workspace = true } indexmap = { workspace = true } keyring = { workspace = true, features = ["crypto-rust"] } libc = { workspace = true } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 5a0a555e8..cebb72a11 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -219,11 +219,7 @@ impl Codex { let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); - let loaded_skills = if config.features.enabled(Feature::Skills) { - Some(skills_manager.skills_for_cwd(&config.cwd)) - } else { - None - }; + let loaded_skills = Some(skills_manager.skills_for_cwd(&config.cwd)); if let Some(outcome) = &loaded_skills { for err in &outcome.errors { @@ -733,30 +729,6 @@ impl Session { // record_initial_history can emit events. We record only after the SessionConfiguredEvent is emitted. sess.record_initial_history(initial_history).await; - if sess.enabled(Feature::Skills) { - let mut rx = sess - .services - .skills_manager - .subscribe_skills_update_notifications(); - let sess = Arc::downgrade(&sess); - tokio::spawn(async move { - loop { - match rx.recv().await { - Ok(()) => { - let Some(sess) = sess.upgrade() else { - break; - }; - let turn_context = sess.new_default_turn().await; - sess.send_event(turn_context.as_ref(), EventMsg::SkillsUpdateAvailable) - .await; - } - Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => continue, - Err(tokio::sync::broadcast::error::RecvError::Closed) => break, - } - } - }); - } - Ok(sess) } @@ -1719,7 +1691,6 @@ mod handlers { use crate::codex::spawn_review_thread; use crate::config::Config; - use crate::features::Feature; use crate::mcp::auth::compute_auth_statuses; use crate::mcp::collect_mcp_snapshot_from_manager; use crate::review_prompts::resolve_review_request; @@ -1999,29 +1970,20 @@ mod handlers { } else { cwds }; - let skills = if sess.enabled(Feature::Skills) { - let skills_manager = &sess.services.skills_manager; - cwds.into_iter() - .map(|cwd| { - let outcome = skills_manager.skills_for_cwd_with_options(&cwd, force_reload); - let errors = super::errors_to_info(&outcome.errors); - let skills = super::skills_to_info(&outcome.skills); - SkillsListEntry { - cwd, - skills, - errors, - } - }) - .collect() - } else { - cwds.into_iter() - .map(|cwd| SkillsListEntry { + let skills_manager = &sess.services.skills_manager; + let skills = cwds + .into_iter() + .map(|cwd| { + let outcome = skills_manager.skills_for_cwd_with_options(&cwd, force_reload); + let errors = super::errors_to_info(&outcome.errors); + let skills = super::skills_to_info(&outcome.skills); + SkillsListEntry { cwd, - skills: Vec::new(), - errors: Vec::new(), - }) - .collect() - }; + skills, + errors, + } + }) + .collect(); let event = Event { id: sub_id, msg: EventMsg::ListSkillsResponse(ListSkillsResponseEvent { skills }), @@ -2266,15 +2228,11 @@ pub(crate) async fn run_task( }); sess.send_event(&turn_context, event).await; - let skills_outcome = if sess.enabled(Feature::Skills) { - Some( - sess.services - .skills_manager - .skills_for_cwd(&turn_context.cwd), - ) - } else { - None - }; + let skills_outcome = Some( + sess.services + .skills_manager + .skills_for_cwd(&turn_context.cwd), + ); let SkillInjections { items: skill_items, diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 3239adc10..1845e26e6 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -79,8 +79,6 @@ pub enum Feature { RemoteModels, /// Allow model to call multiple tools in parallel (only for models supporting it). ParallelToolCalls, - /// Experimental skills injection (CLI flag-driven). - Skills, /// Experimental shell snapshotting. ShellSnapshot, /// Experimental TUI v2 (viewport) implementation. @@ -320,16 +318,6 @@ pub const FEATURES: &[FeatureSpec] = &[ default_enabled: false, }, // Beta program. Rendered in the `/experimental` menu for users. - FeatureSpec { - id: Feature::Skills, - key: "skills", - // stage: Stage::Beta { - // menu_description: "Define new `skills` for the model", - // announcement: "NEW! Try the new `skills` features. Enable in /experimental!", - // }, - stage: Stage::Experimental, - default_enabled: false, - }, FeatureSpec { id: Feature::UnifiedExec, key: "unified_exec", diff --git a/codex-rs/core/src/project_doc.rs b/codex-rs/core/src/project_doc.rs index 208b03556..f115b1295 100644 --- a/codex-rs/core/src/project_doc.rs +++ b/codex-rs/core/src/project_doc.rs @@ -14,7 +14,6 @@ //! 3. We do **not** walk past the Git root. use crate::config::Config; -use crate::features::Feature; use crate::skills::SkillMetadata; use crate::skills::render_skills_section; use dunce::canonicalize as normalize_path; @@ -37,11 +36,7 @@ pub(crate) async fn get_user_instructions( config: &Config, skills: Option<&[SkillMetadata]>, ) -> Option { - let skills_section = if config.features.enabled(Feature::Skills) { - skills.and_then(render_skills_section) - } else { - None - }; + let skills_section = skills.and_then(render_skills_section); let project_docs = match read_project_docs(config).await { Ok(docs) => docs, @@ -260,7 +255,6 @@ mod tests { config.cwd = root.path().to_path_buf(); config.project_doc_max_bytes = limit; - config.features.enable(Feature::Skills); config.user_instructions = instructions.map(ToOwned::to_owned); config diff --git a/codex-rs/core/src/skills/assets/samples/plan/SKILL.md b/codex-rs/core/src/skills/assets/samples/plan/SKILL.md new file mode 100644 index 000000000..5bdfc9bb3 --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/plan/SKILL.md @@ -0,0 +1,169 @@ +--- +name: plan +description: Plan lifecycle management for Codex plans stored in $CODEX_HOME/plans (default ~/.codex/plans). Use when a user asks to create, find, read, update, delete, or manage plan documents for implementation work or overview/reference documentation. +--- + +# Plan + +## Overview + +Create and manage plan documents on disk. Plans stored on disk are markdown files with YAML frontmatter and free-form content. When drafting in chat, output only the plan body without frontmatter; add frontmatter only when stashing to disk. Support both implementation plans and overview/reference plans. Only write to the plans folder; do not modify the repository codebase. + +## Core rules + +- Resolve the plans directory as `$CODEX_HOME/plans` or `~/.codex/plans` when `CODEX_HOME` is not set. +- Create the plans directory if it does not exist. +- Never write to the repo; only read files to understand context. +- Require frontmatter with **only** `name` and `description` (single-line values) for on-disk plans. +- When presenting a draft plan in chat, omit frontmatter and start at `# Plan`. +- Enforce naming rules: short, lower-case, hyphen-delimited; filename must equal `.md`. +- If a plan is not found, state it clearly and offer to create one. +- Allow overview-style plans that document flows, architecture, or context without a work checklist. + +## Decide the task + +1. **Find/list**: discover plans by frontmatter summary; confirm if multiple matches exist. +2. **Read/use**: validate frontmatter; present summary and full contents. +3. **Create**: inspect repo read-only; choose plan style (implementation vs overview); draft plan; write to plans directory only. +4. **Update**: load plan; revise content and/or description; preserve frontmatter keys; overwrite the plan file. +5. **Delete**: confirm intent, then remove the plan file if asked. + +## Plan discovery + +- Prefer `scripts/list_plans.py` for quick summaries. +- Use `scripts/read_plan_frontmatter.py` to validate a specific plan. +- If name mismatches filename or frontmatter is missing fields, call it out and ask whether to fix. + +## Plan creation workflow + +1. Read relevant docs and entry points (`README.md`, `docs/`, key modules) to scope requirements. +2. Identify scope, constraints, and data model/API implications (or capture existing behavior for an overview). +3. Draft either an ordered implementation plan or a structured overview plan with diagrams/notes as needed. +4. Immediately output the plan body only (no frontmatter), then ask the user if they want to 1. Make changes, 2. Implement it, 3. Stash it as per plan. +5. If the user wants to stash it, prepend frontmatter and save the plan under the computed plans directory using `scripts/create_plan.py`. + +## Plan update workflow + +- Re-read the plan and related code/docs before updating. +- Keep the plan name stable unless the user explicitly wants a rename. +- If renaming, update both frontmatter `name` and filename together. + +## Scripts (low-freedom helpers) + +Create a plan file (body only; frontmatter is written for you). Run from the plan skill directory: + +```bash +python ./scripts/create_plan.py \ + --name codex-rate-limit-overview \ + --description "Scope and update plan for Codex rate limiting" \ + --body-file /tmp/plan-body.md +``` + +Read frontmatter summary for a plan (run from the plan skill directory): + +```bash +python ./scripts/read_plan_frontmatter.py ~/.codex/plans/codex-rate-limit-overview.md +``` + +List plan summaries (optional filter; run from the plan skill directory): + +```bash +python ./scripts/list_plans.py --query "rate limit" +``` + +## Plan file format + +Use one of the structures below for the plan body. When drafting, output only the body (no frontmatter). When stashing, prepend this frontmatter: + +```markdown +--- +name: +description: <1-line summary> +--- +``` + +### Implementation plan body template + +```markdown +# Plan + +<1-3 sentences: intent, scope, and approach.> + +## Requirements +- +- + +## Scope +- In: +- Out: + +## Files and entry points +- +- + +## Data model / API changes +- + +## Action items +[ ] +[ ] +[ ] +[ ] +[ ] +[ ] + +## Testing and validation +- + +## Risks and edge cases +- +- + +## Open questions +- +- +``` + +### Overview plan body template + +```markdown +# Plan + +<1-3 sentences: intent and scope of the overview.> + +## Overview + + +## Diagrams + + +## Key file references +- +- + +## Auth / routing / behavior notes +- + +## Current status +- + +## Action items +- None (overview only). + +## Testing and validation +- None (overview only). + +## Risks and edge cases +- None (overview only). + +## Open questions +- None. +``` + +## Writing guidance + +- Keep action items ordered and concrete; include file/entry-point hints. +- For overview plans, keep action items minimal and set sections to "None" when not applicable. +- Always include testing/validation and risks/edge cases in implementation plans. +- Use open questions only when necessary (max 3). +- If a section is not applicable, note "None" briefly rather than removing it. diff --git a/codex-rs/core/src/skills/assets/samples/plan/scripts/create_plan.py b/codex-rs/core/src/skills/assets/samples/plan/scripts/create_plan.py new file mode 100644 index 000000000..4cbfa8f5e --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/plan/scripts/create_plan.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +"""Create or overwrite a plan markdown file in $CODEX_HOME/plans.""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +from plan_utils import get_plans_dir, validate_plan_name + +DEFAULT_TEMPLATE = """# Plan + +<1-3 sentences: intent, scope, and approach.> + +## Requirements +- +- + +## Scope +- In: +- Out: + +## Files and entry points +- +- + +## Data model / API changes +- + +## Action items +[ ] +[ ] +[ ] +[ ] +[ ] +[ ] + +## Testing and validation +- + +## Risks and edge cases +- +- + +## Open questions +- +- +""" + + +def read_body(args: argparse.Namespace) -> str | None: + if args.template: + return DEFAULT_TEMPLATE + if args.body_file: + return Path(args.body_file).read_text(encoding="utf-8") + if not sys.stdin.isatty(): + return sys.stdin.read() + return None + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Create a plan file under $CODEX_HOME/plans or ~/.codex/plans." + ) + parser.add_argument("--name", required=True, help="Plan name (lower-case, hyphen-delimited).") + parser.add_argument("--description", required=True, help="Short plan description.") + parser.add_argument( + "--body-file", + help="Path to markdown body (without frontmatter). If omitted, read from stdin.", + ) + parser.add_argument( + "--template", + action="store_true", + help="Write a template body instead of reading from stdin or --body-file.", + ) + parser.add_argument( + "--overwrite", + action="store_true", + help="Overwrite the plan file if it already exists.", + ) + args = parser.parse_args() + + name = args.name.strip() + description = args.description.strip() + validate_plan_name(name) + if not description or "\n" in description: + raise SystemExit("Description must be a single line.") + + body = read_body(args) + if body is None: + raise SystemExit("Provide --body-file, stdin, or --template to supply plan content.") + + body = body.strip() + if not body: + raise SystemExit("Plan body cannot be empty.") + if body.lstrip().startswith("---"): + raise SystemExit("Plan body should not include frontmatter.") + + plans_dir = get_plans_dir() + plans_dir.mkdir(parents=True, exist_ok=True) + plan_path = plans_dir / f"{name}.md" + + if plan_path.exists() and not args.overwrite: + raise SystemExit(f"Plan already exists: {plan_path}. Use --overwrite to replace.") + + content = f"---\nname: {name}\ndescription: {description}\n---\n\n{body}\n" + plan_path.write_text(content, encoding="utf-8") + print(str(plan_path)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/codex-rs/core/src/skills/assets/samples/plan/scripts/list_plans.py b/codex-rs/core/src/skills/assets/samples/plan/scripts/list_plans.py new file mode 100644 index 000000000..db6c412dc --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/plan/scripts/list_plans.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 +"""List plan summaries by reading frontmatter only.""" + +from __future__ import annotations + +import argparse +import json + +from plan_utils import get_plans_dir, parse_frontmatter + + +def main() -> int: + parser = argparse.ArgumentParser(description="List plan summaries from $CODEX_HOME/plans.") + parser.add_argument("--query", help="Case-insensitive substring to filter name/description.") + parser.add_argument("--json", action="store_true", help="Emit JSON output.") + args = parser.parse_args() + + plans_dir = get_plans_dir() + if not plans_dir.exists(): + raise SystemExit(f"Plans directory not found: {plans_dir}") + + query = args.query.lower() if args.query else None + items = [] + for path in sorted(plans_dir.glob("*.md")): + try: + data = parse_frontmatter(path) + except ValueError: + continue + name = data.get("name") + description = data.get("description") + if not name or not description: + continue + if query: + haystack = f"{name} {description}".lower() + if query not in haystack: + continue + items.append({"name": name, "description": description, "path": str(path)}) + + if args.json: + print(json.dumps(items)) + else: + for item in items: + print(f"{item['name']}\t{item['description']}\t{item['path']}") + + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/codex-rs/core/src/skills/assets/samples/plan/scripts/plan_utils.py b/codex-rs/core/src/skills/assets/samples/plan/scripts/plan_utils.py new file mode 100644 index 000000000..1a36a4bf2 --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/plan/scripts/plan_utils.py @@ -0,0 +1,53 @@ +#!/usr/bin/env python3 +"""Shared helpers for plan scripts.""" + +from __future__ import annotations + +import os +import re +from pathlib import Path + +_NAME_RE = re.compile(r"^[a-z0-9]+(-[a-z0-9]+)*$") + + +def get_codex_home() -> Path: + """Return CODEX_HOME if set, else ~/.codex.""" + return Path(os.environ.get("CODEX_HOME", "~/.codex")).expanduser() + + +def get_plans_dir() -> Path: + return get_codex_home() / "plans" + + +def validate_plan_name(name: str) -> None: + if not name or not _NAME_RE.match(name): + raise ValueError( + "Invalid plan name. Use short, lower-case, hyphen-delimited names " + "(e.g., codex-rate-limit-overview)." + ) + + +def parse_frontmatter(path: Path) -> dict: + """Parse YAML frontmatter from a markdown file without reading the body.""" + with path.open("r", encoding="utf-8") as handle: + first = handle.readline() + if first.strip() != "---": + raise ValueError("Frontmatter must start with '---'.") + + data: dict[str, str] = {} + for line in handle: + stripped = line.strip() + if stripped == "---": + return data + if not stripped or stripped.startswith("#"): + continue + if ":" not in line: + raise ValueError(f"Invalid frontmatter line: {line.rstrip()}") + key, value = line.split(":", 1) + key = key.strip() + value = value.strip() + if value and len(value) >= 2 and value[0] == value[-1] and value[0] in ('"', "'"): + value = value[1:-1] + data[key] = value + + raise ValueError("Frontmatter must end with '---'.") diff --git a/codex-rs/core/src/skills/assets/samples/plan/scripts/read_plan_frontmatter.py b/codex-rs/core/src/skills/assets/samples/plan/scripts/read_plan_frontmatter.py new file mode 100644 index 000000000..1c881ee01 --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/plan/scripts/read_plan_frontmatter.py @@ -0,0 +1,41 @@ +#!/usr/bin/env python3 +"""Read plan frontmatter without loading the full markdown body.""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path + +from plan_utils import parse_frontmatter + + +def main() -> int: + parser = argparse.ArgumentParser(description="Read name/description from plan frontmatter.") + parser.add_argument("plan_path", help="Path to the plan markdown file.") + parser.add_argument("--json", action="store_true", help="Emit JSON output.") + args = parser.parse_args() + + path = Path(args.plan_path).expanduser() + if not path.exists(): + raise SystemExit(f"Plan not found: {path}") + + data = parse_frontmatter(path) + name = data.get("name") + description = data.get("description") + if not name or not description: + raise SystemExit("Frontmatter must include name and description.") + + payload = {"name": name, "description": description, "path": str(path)} + if args.json: + print(json.dumps(payload)) + else: + print(f"name: {name}") + print(f"description: {description}") + print(f"path: {path}") + + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/SKILL.md b/codex-rs/core/src/skills/assets/samples/skill-creator/SKILL.md new file mode 100644 index 000000000..64f076f18 --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/skill-creator/SKILL.md @@ -0,0 +1,362 @@ +--- +name: Skill Creator +description: Guide for creating effective skills. This skill should be used when users want to create a new skill (or update an existing skill) that extends Codex's capabilities with specialized knowledge, workflows, or tool integrations. +--- + +# Skill Creator + +This skill provides guidance for creating effective skills. + +## About Skills + +Skills are modular, self-contained packages that extend Codex's capabilities by providing +specialized knowledge, workflows, and tools. Think of them as "onboarding guides" for specific +domains or tasks—they transform Codex from a general-purpose agent into a specialized agent +equipped with procedural knowledge that no model can fully possess. + +### What Skills Provide + +1. Specialized workflows - Multi-step procedures for specific domains +2. Tool integrations - Instructions for working with specific file formats or APIs +3. Domain expertise - Company-specific knowledge, schemas, business logic +4. Bundled resources - Scripts, references, and assets for complex and repetitive tasks + +## Core Principles + +### Concise is Key + +The context window is a public good. Skills share the context window with everything else Codex needs: system prompt, conversation history, other Skills' metadata, and the actual user request. + +**Default assumption: Codex is already very smart.** Only add context Codex doesn't already have. Challenge each piece of information: "Does Codex really need this explanation?" and "Does this paragraph justify its token cost?" + +Prefer concise examples over verbose explanations. + +### Set Appropriate Degrees of Freedom + +Match the level of specificity to the task's fragility and variability: + +**High freedom (text-based instructions)**: Use when multiple approaches are valid, decisions depend on context, or heuristics guide the approach. + +**Medium freedom (pseudocode or scripts with parameters)**: Use when a preferred pattern exists, some variation is acceptable, or configuration affects behavior. + +**Low freedom (specific scripts, few parameters)**: Use when operations are fragile and error-prone, consistency is critical, or a specific sequence must be followed. + +Think of Codex as exploring a path: a narrow bridge with cliffs needs specific guardrails (low freedom), while an open field allows many routes (high freedom). + +### Anatomy of a Skill + +Every skill consists of a required SKILL.md file and optional bundled resources: + +``` +skill-name/ +├── SKILL.md (required) +│ ├── YAML frontmatter metadata (required) +│ │ ├── name: (required) +│ │ └── description: (required) +│ └── Markdown instructions (required) +└── Bundled Resources (optional) + ├── scripts/ - Executable code (Python/Bash/etc.) + ├── references/ - Documentation intended to be loaded into context as needed + └── assets/ - Files used in output (templates, icons, fonts, etc.) +``` + +#### SKILL.md (required) + +Every SKILL.md consists of: + +- **Frontmatter** (YAML): Contains `name` and `description` fields. These are the only fields that Codex reads to determine when the skill gets used, thus it is very important to be clear and comprehensive in describing what the skill is, and when it should be used. +- **Body** (Markdown): Instructions and guidance for using the skill. Only loaded AFTER the skill triggers (if at all). + +#### Bundled Resources (optional) + +##### Scripts (`scripts/`) + +Executable code (Python/Bash/etc.) for tasks that require deterministic reliability or are repeatedly rewritten. + +- **When to include**: When the same code is being rewritten repeatedly or deterministic reliability is needed +- **Example**: `scripts/rotate_pdf.py` for PDF rotation tasks +- **Benefits**: Token efficient, deterministic, may be executed without loading into context +- **Note**: Scripts may still need to be read by Codex for patching or environment-specific adjustments + +##### References (`references/`) + +Documentation and reference material intended to be loaded as needed into context to inform Codex's process and thinking. + +- **When to include**: For documentation that Codex should reference while working +- **Examples**: `references/finance.md` for financial schemas, `references/mnda.md` for company NDA template, `references/policies.md` for company policies, `references/api_docs.md` for API specifications +- **Use cases**: Database schemas, API documentation, domain knowledge, company policies, detailed workflow guides +- **Benefits**: Keeps SKILL.md lean, loaded only when Codex determines it's needed +- **Best practice**: If files are large (>10k words), include grep search patterns in SKILL.md +- **Avoid duplication**: Information should live in either SKILL.md or references files, not both. Prefer references files for detailed information unless it's truly core to the skill—this keeps SKILL.md lean while making information discoverable without hogging the context window. Keep only essential procedural instructions and workflow guidance in SKILL.md; move detailed reference material, schemas, and examples to references files. + +##### Assets (`assets/`) + +Files not intended to be loaded into context, but rather used within the output Codex produces. + +- **When to include**: When the skill needs files that will be used in the final output +- **Examples**: `assets/logo.png` for brand assets, `assets/slides.pptx` for PowerPoint templates, `assets/frontend-template/` for HTML/React boilerplate, `assets/font.ttf` for typography +- **Use cases**: Templates, images, icons, boilerplate code, fonts, sample documents that get copied or modified +- **Benefits**: Separates output resources from documentation, enables Codex to use files without loading them into context + +#### What to Not Include in a Skill + +A skill should only contain essential files that directly support its functionality. Do NOT create extraneous documentation or auxiliary files, including: + +- README.md +- INSTALLATION_GUIDE.md +- QUICK_REFERENCE.md +- CHANGELOG.md +- etc. + +The skill should only contain the information needed for an AI agent to do the job at hand. It should not contain auxiliary context about the process that went into creating it, setup and testing procedures, user-facing documentation, etc. Creating additional documentation files just adds clutter and confusion. + +### Progressive Disclosure Design Principle + +Skills use a three-level loading system to manage context efficiently: + +1. **Metadata (name + description)** - Always in context (~100 words) +2. **SKILL.md body** - When skill triggers (<5k words) +3. **Bundled resources** - As needed by Codex (Unlimited because scripts can be executed without reading into context window) + +#### Progressive Disclosure Patterns + +Keep SKILL.md body to the essentials and under 500 lines to minimize context bloat. Split content into separate files when approaching this limit. When splitting out content into other files, it is very important to reference them from SKILL.md and describe clearly when to read them, to ensure the reader of the skill knows they exist and when to use them. + +**Key principle:** When a skill supports multiple variations, frameworks, or options, keep only the core workflow and selection guidance in SKILL.md. Move variant-specific details (patterns, examples, configuration) into separate reference files. + +**Pattern 1: High-level guide with references** + +```markdown +# PDF Processing + +## Quick start + +Extract text with pdfplumber: +[code example] + +## Advanced features + +- **Form filling**: See [FORMS.md](FORMS.md) for complete guide +- **API reference**: See [REFERENCE.md](REFERENCE.md) for all methods +- **Examples**: See [EXAMPLES.md](EXAMPLES.md) for common patterns +``` + +Codex loads FORMS.md, REFERENCE.md, or EXAMPLES.md only when needed. + +**Pattern 2: Domain-specific organization** + +For Skills with multiple domains, organize content by domain to avoid loading irrelevant context: + +``` +bigquery-skill/ +├── SKILL.md (overview and navigation) +└── reference/ + ├── finance.md (revenue, billing metrics) + ├── sales.md (opportunities, pipeline) + ├── product.md (API usage, features) + └── marketing.md (campaigns, attribution) +``` + +When a user asks about sales metrics, Codex only reads sales.md. + +Similarly, for skills supporting multiple frameworks or variants, organize by variant: + +``` +cloud-deploy/ +├── SKILL.md (workflow + provider selection) +└── references/ + ├── aws.md (AWS deployment patterns) + ├── gcp.md (GCP deployment patterns) + └── azure.md (Azure deployment patterns) +``` + +When the user chooses AWS, Codex only reads aws.md. + +**Pattern 3: Conditional details** + +Show basic content, link to advanced content: + +```markdown +# DOCX Processing + +## Creating documents + +Use docx-js for new documents. See [DOCX-JS.md](DOCX-JS.md). + +## Editing documents + +For simple edits, modify the XML directly. + +**For tracked changes**: See [REDLINING.md](REDLINING.md) +**For OOXML details**: See [OOXML.md](OOXML.md) +``` + +Codex reads REDLINING.md or OOXML.md only when the user needs those features. + +**Important guidelines:** + +- **Avoid deeply nested references** - Keep references one level deep from SKILL.md. All reference files should link directly from SKILL.md. +- **Structure longer reference files** - For files longer than 100 lines, include a table of contents at the top so Codex can see the full scope when previewing. + +## Skill Creation Process + +Skill creation involves these steps: + +1. Understand the skill with concrete examples +2. Plan reusable skill contents (scripts, references, assets) +3. Initialize the skill (run init_skill.py) +4. Edit the skill (implement resources and write SKILL.md) +5. Package the skill (run package_skill.py) +6. Iterate based on real usage + +Follow these steps in order, skipping only if there is a clear reason why they are not applicable. + +### Skill Naming + +- Use lowercase letters, digits, and hyphens only; normalize user-provided titles to hyphen-case (e.g., "Plan Mode" -> `plan-mode`). +- Prefer short, verb-led phrases that describe the action. +- Namespace by tool when it improves clarity or triggering (e.g., `gh-address-comments`, `linear-address-issue`). +- Name the skill folder exactly after the skill name. + +### Step 1: Understanding the Skill with Concrete Examples + +Skip this step only when the skill's usage patterns are already clearly understood. It remains valuable even when working with an existing skill. + +To create an effective skill, clearly understand concrete examples of how the skill will be used. This understanding can come from either direct user examples or generated examples that are validated with user feedback. + +For example, when building an image-editor skill, relevant questions include: + +- "What functionality should the image-editor skill support? Editing, rotating, anything else?" +- "Can you give some examples of how this skill would be used?" +- "I can imagine users asking for things like 'Remove the red-eye from this image' or 'Rotate this image'. Are there other ways you imagine this skill being used?" +- "What would a user say that should trigger this skill?" + +To avoid overwhelming users, avoid asking too many questions in a single message. Start with the most important questions and follow up as needed for better effectiveness. + +Conclude this step when there is a clear sense of the functionality the skill should support. + +### Step 2: Planning the Reusable Skill Contents + +To turn concrete examples into an effective skill, analyze each example by: + +1. Considering how to execute on the example from scratch +2. Identifying what scripts, references, and assets would be helpful when executing these workflows repeatedly + +Example: When building a `pdf-editor` skill to handle queries like "Help me rotate this PDF," the analysis shows: + +1. Rotating a PDF requires re-writing the same code each time +2. A `scripts/rotate_pdf.py` script would be helpful to store in the skill + +Example: When designing a `frontend-webapp-builder` skill for queries like "Build me a todo app" or "Build me a dashboard to track my steps," the analysis shows: + +1. Writing a frontend webapp requires the same boilerplate HTML/React each time +2. An `assets/hello-world/` template containing the boilerplate HTML/React project files would be helpful to store in the skill + +Example: When building a `big-query` skill to handle queries like "How many users have logged in today?" the analysis shows: + +1. Querying BigQuery requires re-discovering the table schemas and relationships each time +2. A `references/schema.md` file documenting the table schemas would be helpful to store in the skill + +To establish the skill's contents, analyze each concrete example to create a list of the reusable resources to include: scripts, references, and assets. + +### Step 3: Initializing the Skill + +At this point, it is time to actually create the skill. + +Skip this step only if the skill being developed already exists, and iteration or packaging is needed. In this case, continue to the next step. + +When creating a new skill from scratch, always run the `init_skill.py` script. The script conveniently generates a new template skill directory that automatically includes everything a skill requires, making the skill creation process much more efficient and reliable. + +Usage: + +```bash +scripts/init_skill.py --path +``` + +The script: + +- Creates the skill directory at the specified path +- Generates a SKILL.md template with proper frontmatter and TODO placeholders +- Creates example resource directories: `scripts/`, `references/`, and `assets/` +- Adds example files in each directory that can be customized or deleted + +After initialization, customize or remove the generated SKILL.md and example files as needed. + +### Step 4: Edit the Skill + +When editing the (newly-generated or existing) skill, remember that the skill is being created for another instance of Codex to use. Include information that would be beneficial and non-obvious to Codex. Consider what procedural knowledge, domain-specific details, or reusable assets would help another Codex instance execute these tasks more effectively. + +#### Learn Proven Design Patterns + +Consult these helpful guides based on your skill's needs: + +- **Multi-step processes**: See references/workflows.md for sequential workflows and conditional logic +- **Specific output formats or quality standards**: See references/output-patterns.md for template and example patterns + +These files contain established best practices for effective skill design. + +#### Start with Reusable Skill Contents + +To begin implementation, start with the reusable resources identified above: `scripts/`, `references/`, and `assets/` files. Note that this step may require user input. For example, when implementing a `brand-guidelines` skill, the user may need to provide brand assets or templates to store in `assets/`, or documentation to store in `references/`. + +Added scripts must be tested by actually running them to ensure there are no bugs and that the output matches what is expected. If there are many similar scripts, only a representative sample needs to be tested to ensure confidence that they all work while balancing time to completion. + +Any example files and directories not needed for the skill should be deleted. The initialization script creates example files in `scripts/`, `references/`, and `assets/` to demonstrate structure, but most skills won't need all of them. + +#### Update SKILL.md + +**Writing Guidelines:** Always use imperative/infinitive form. + +##### Frontmatter + +Write the YAML frontmatter with `name` and `description`: + +- `name`: The skill name +- `description`: This is the primary triggering mechanism for your skill, and helps Codex understand when to use the skill. + - Include both what the Skill does and specific triggers/contexts for when to use it. + - Include all "when to use" information here - Not in the body. The body is only loaded after triggering, so "When to Use This Skill" sections in the body are not helpful to Codex. + - Example description for a `docx` skill: "Comprehensive document creation, editing, and analysis with support for tracked changes, comments, formatting preservation, and text extraction. Use when Codex needs to work with professional documents (.docx files) for: (1) Creating new documents, (2) Modifying or editing content, (3) Working with tracked changes, (4) Adding comments, or any other document tasks" + +Do not include any other fields in YAML frontmatter. + +##### Body + +Write instructions for using the skill and its bundled resources. + +### Step 5: Packaging a Skill + +Once development of the skill is complete, it must be packaged into a distributable .skill file that gets shared with the user. The packaging process automatically validates the skill first to ensure it meets all requirements: + +```bash +scripts/package_skill.py +``` + +Optional output directory specification: + +```bash +scripts/package_skill.py ./dist +``` + +The packaging script will: + +1. **Validate** the skill automatically, checking: + + - YAML frontmatter format and required fields + - Skill naming conventions and directory structure + - Description completeness and quality + - File organization and resource references + +2. **Package** the skill if validation passes, creating a .skill file named after the skill (e.g., `my-skill.skill`) that includes all files and maintains the proper directory structure for distribution. The .skill file is a zip file with a .skill extension. + +If validation fails, the script will report the errors and exit without creating a package. Fix any validation errors and run the packaging command again. + +### Step 6: Iterate + +After testing the skill, users may request improvements. Often this happens right after using the skill, with fresh context of how the skill performed. + +**Iteration workflow:** + +1. Use the skill on real tasks +2. Notice struggles or inefficiencies +3. Identify how SKILL.md or bundled resources should be updated +4. Implement changes and test again diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/license.txt b/codex-rs/core/src/skills/assets/samples/skill-creator/license.txt new file mode 100644 index 000000000..d64569567 --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/skill-creator/license.txt @@ -0,0 +1,202 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/init_skill.py b/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/init_skill.py new file mode 100644 index 000000000..2f49f0191 --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/init_skill.py @@ -0,0 +1,327 @@ +#!/usr/bin/env python3 +""" +Skill Initializer - Creates a new skill from template + +Usage: + init_skill.py --path + +Examples: + init_skill.py my-new-skill --path skills/public + init_skill.py my-api-helper --path skills/private + init_skill.py custom-skill --path /custom/location +""" + +import re +import sys +from pathlib import Path + +MAX_SKILL_NAME_LENGTH = 64 + +SKILL_TEMPLATE = """--- +name: {skill_name} +description: [TODO: Complete and informative explanation of what the skill does and when to use it. Include WHEN to use this skill - specific scenarios, file types, or tasks that trigger it.] +--- + +# {skill_title} + +## Overview + +[TODO: 1-2 sentences explaining what this skill enables] + +## Structuring This Skill + +[TODO: Choose the structure that best fits this skill's purpose. Common patterns: + +**1. Workflow-Based** (best for sequential processes) +- Works well when there are clear step-by-step procedures +- Example: DOCX skill with "Workflow Decision Tree" → "Reading" → "Creating" → "Editing" +- Structure: ## Overview → ## Workflow Decision Tree → ## Step 1 → ## Step 2... + +**2. Task-Based** (best for tool collections) +- Works well when the skill offers different operations/capabilities +- Example: PDF skill with "Quick Start" → "Merge PDFs" → "Split PDFs" → "Extract Text" +- Structure: ## Overview → ## Quick Start → ## Task Category 1 → ## Task Category 2... + +**3. Reference/Guidelines** (best for standards or specifications) +- Works well for brand guidelines, coding standards, or requirements +- Example: Brand styling with "Brand Guidelines" → "Colors" → "Typography" → "Features" +- Structure: ## Overview → ## Guidelines → ## Specifications → ## Usage... + +**4. Capabilities-Based** (best for integrated systems) +- Works well when the skill provides multiple interrelated features +- Example: Product Management with "Core Capabilities" → numbered capability list +- Structure: ## Overview → ## Core Capabilities → ### 1. Feature → ### 2. Feature... + +Patterns can be mixed and matched as needed. Most skills combine patterns (e.g., start with task-based, add workflow for complex operations). + +Delete this entire "Structuring This Skill" section when done - it's just guidance.] + +## [TODO: Replace with the first main section based on chosen structure] + +[TODO: Add content here. See examples in existing skills: +- Code samples for technical skills +- Decision trees for complex workflows +- Concrete examples with realistic user requests +- References to scripts/templates/references as needed] + +## Resources + +This skill includes example resource directories that demonstrate how to organize different types of bundled resources: + +### scripts/ +Executable code (Python/Bash/etc.) that can be run directly to perform specific operations. + +**Examples from other skills:** +- PDF skill: `fill_fillable_fields.py`, `extract_form_field_info.py` - utilities for PDF manipulation +- DOCX skill: `document.py`, `utilities.py` - Python modules for document processing + +**Appropriate for:** Python scripts, shell scripts, or any executable code that performs automation, data processing, or specific operations. + +**Note:** Scripts may be executed without loading into context, but can still be read by Codex for patching or environment adjustments. + +### references/ +Documentation and reference material intended to be loaded into context to inform Codex's process and thinking. + +**Examples from other skills:** +- Product management: `communication.md`, `context_building.md` - detailed workflow guides +- BigQuery: API reference documentation and query examples +- Finance: Schema documentation, company policies + +**Appropriate for:** In-depth documentation, API references, database schemas, comprehensive guides, or any detailed information that Codex should reference while working. + +### assets/ +Files not intended to be loaded into context, but rather used within the output Codex produces. + +**Examples from other skills:** +- Brand styling: PowerPoint template files (.pptx), logo files +- Frontend builder: HTML/React boilerplate project directories +- Typography: Font files (.ttf, .woff2) + +**Appropriate for:** Templates, boilerplate code, document templates, images, icons, fonts, or any files meant to be copied or used in the final output. + +--- + +**Any unneeded directories can be deleted.** Not every skill requires all three types of resources. +""" + +EXAMPLE_SCRIPT = '''#!/usr/bin/env python3 +""" +Example helper script for {skill_name} + +This is a placeholder script that can be executed directly. +Replace with actual implementation or delete if not needed. + +Example real scripts from other skills: +- pdf/scripts/fill_fillable_fields.py - Fills PDF form fields +- pdf/scripts/convert_pdf_to_images.py - Converts PDF pages to images +""" + +def main(): + print("This is an example script for {skill_name}") + # TODO: Add actual script logic here + # This could be data processing, file conversion, API calls, etc. + +if __name__ == "__main__": + main() +''' + +EXAMPLE_REFERENCE = """# Reference Documentation for {skill_title} + +This is a placeholder for detailed reference documentation. +Replace with actual reference content or delete if not needed. + +Example real reference docs from other skills: +- product-management/references/communication.md - Comprehensive guide for status updates +- product-management/references/context_building.md - Deep-dive on gathering context +- bigquery/references/ - API references and query examples + +## When Reference Docs Are Useful + +Reference docs are ideal for: +- Comprehensive API documentation +- Detailed workflow guides +- Complex multi-step processes +- Information too lengthy for main SKILL.md +- Content that's only needed for specific use cases + +## Structure Suggestions + +### API Reference Example +- Overview +- Authentication +- Endpoints with examples +- Error codes +- Rate limits + +### Workflow Guide Example +- Prerequisites +- Step-by-step instructions +- Common patterns +- Troubleshooting +- Best practices +""" + +EXAMPLE_ASSET = """# Example Asset File + +This placeholder represents where asset files would be stored. +Replace with actual asset files (templates, images, fonts, etc.) or delete if not needed. + +Asset files are NOT intended to be loaded into context, but rather used within +the output Codex produces. + +Example asset files from other skills: +- Brand guidelines: logo.png, slides_template.pptx +- Frontend builder: hello-world/ directory with HTML/React boilerplate +- Typography: custom-font.ttf, font-family.woff2 +- Data: sample_data.csv, test_dataset.json + +## Common Asset Types + +- Templates: .pptx, .docx, boilerplate directories +- Images: .png, .jpg, .svg, .gif +- Fonts: .ttf, .otf, .woff, .woff2 +- Boilerplate code: Project directories, starter files +- Icons: .ico, .svg +- Data files: .csv, .json, .xml, .yaml + +Note: This is a text placeholder. Actual assets can be any file type. +""" + + +def normalize_skill_name(skill_name): + """Normalize a skill name to lowercase hyphen-case.""" + normalized = skill_name.strip().lower() + normalized = re.sub(r"[^a-z0-9]+", "-", normalized) + normalized = normalized.strip("-") + normalized = re.sub(r"-{2,}", "-", normalized) + return normalized + + +def title_case_skill_name(skill_name): + """Convert hyphenated skill name to Title Case for display.""" + return " ".join(word.capitalize() for word in skill_name.split("-")) + + +def init_skill(skill_name, path): + """ + Initialize a new skill directory with template SKILL.md. + + Args: + skill_name: Name of the skill + path: Path where the skill directory should be created + + Returns: + Path to created skill directory, or None if error + """ + # Determine skill directory path + skill_dir = Path(path).resolve() / skill_name + + # Check if directory already exists + if skill_dir.exists(): + print(f"❌ Error: Skill directory already exists: {skill_dir}") + return None + + # Create skill directory + try: + skill_dir.mkdir(parents=True, exist_ok=False) + print(f"✅ Created skill directory: {skill_dir}") + except Exception as e: + print(f"❌ Error creating directory: {e}") + return None + + # Create SKILL.md from template + skill_title = title_case_skill_name(skill_name) + skill_content = SKILL_TEMPLATE.format(skill_name=skill_name, skill_title=skill_title) + + skill_md_path = skill_dir / "SKILL.md" + try: + skill_md_path.write_text(skill_content) + print("✅ Created SKILL.md") + except Exception as e: + print(f"❌ Error creating SKILL.md: {e}") + return None + + # Create resource directories with example files + try: + # Create scripts/ directory with example script + scripts_dir = skill_dir / "scripts" + scripts_dir.mkdir(exist_ok=True) + example_script = scripts_dir / "example.py" + example_script.write_text(EXAMPLE_SCRIPT.format(skill_name=skill_name)) + example_script.chmod(0o755) + print("✅ Created scripts/example.py") + + # Create references/ directory with example reference doc + references_dir = skill_dir / "references" + references_dir.mkdir(exist_ok=True) + example_reference = references_dir / "api_reference.md" + example_reference.write_text(EXAMPLE_REFERENCE.format(skill_title=skill_title)) + print("✅ Created references/api_reference.md") + + # Create assets/ directory with example asset placeholder + assets_dir = skill_dir / "assets" + assets_dir.mkdir(exist_ok=True) + example_asset = assets_dir / "example_asset.txt" + example_asset.write_text(EXAMPLE_ASSET) + print("✅ Created assets/example_asset.txt") + except Exception as e: + print(f"❌ Error creating resource directories: {e}") + return None + + # Print next steps + print(f"\n✅ Skill '{skill_name}' initialized successfully at {skill_dir}") + print("\nNext steps:") + print("1. Edit SKILL.md to complete the TODO items and update the description") + print("2. Customize or delete the example files in scripts/, references/, and assets/") + print("3. Run the validator when ready to check the skill structure") + + return skill_dir + + +def main(): + if len(sys.argv) < 4 or sys.argv[2] != "--path": + print("Usage: init_skill.py --path ") + print("\nSkill name requirements:") + print(" - Use a hyphen-case identifier (e.g., 'data-analyzer')") + print( + " - Input is normalized to lowercase letters, digits, and hyphens only " + "(e.g., 'Plan Mode' -> 'plan-mode')" + ) + print(f" - Max {MAX_SKILL_NAME_LENGTH} characters after normalization") + print(" - Directory name matches the normalized skill name") + print("\nExamples:") + print(" init_skill.py my-new-skill --path skills/public") + print(" init_skill.py my-api-helper --path skills/private") + print(" init_skill.py custom-skill --path /custom/location") + sys.exit(1) + + raw_skill_name = sys.argv[1] + skill_name = normalize_skill_name(raw_skill_name) + if not skill_name: + print("❌ Error: Skill name must include at least one letter or digit.") + sys.exit(1) + if len(skill_name) > MAX_SKILL_NAME_LENGTH: + print( + f"❌ Error: Skill name '{skill_name}' is too long ({len(skill_name)} characters). " + f"Maximum is {MAX_SKILL_NAME_LENGTH} characters." + ) + sys.exit(1) + if skill_name != raw_skill_name: + print(f"Note: Normalized skill name from '{raw_skill_name}' to '{skill_name}'.") + + path = sys.argv[3] + + print(f"🚀 Initializing skill: {skill_name}") + print(f" Location: {path}") + print() + + result = init_skill(skill_name, path) + + if result: + sys.exit(0) + else: + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/package_skill.py b/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/package_skill.py new file mode 100644 index 000000000..4214dc9ac --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/package_skill.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python3 +""" +Skill Packager - Creates a distributable .skill file of a skill folder + +Usage: + python utils/package_skill.py [output-directory] + +Example: + python utils/package_skill.py skills/public/my-skill + python utils/package_skill.py skills/public/my-skill ./dist +""" + +import sys +import zipfile +from pathlib import Path + +from quick_validate import validate_skill + + +def package_skill(skill_path, output_dir=None): + """ + Package a skill folder into a .skill file. + + Args: + skill_path: Path to the skill folder + output_dir: Optional output directory for the .skill file (defaults to current directory) + + Returns: + Path to the created .skill file, or None if error + """ + skill_path = Path(skill_path).resolve() + + # Validate skill folder exists + if not skill_path.exists(): + print(f"❌ Error: Skill folder not found: {skill_path}") + return None + + if not skill_path.is_dir(): + print(f"❌ Error: Path is not a directory: {skill_path}") + return None + + # Validate SKILL.md exists + skill_md = skill_path / "SKILL.md" + if not skill_md.exists(): + print(f"❌ Error: SKILL.md not found in {skill_path}") + return None + + # Run validation before packaging + print("🔍 Validating skill...") + valid, message = validate_skill(skill_path) + if not valid: + print(f"❌ Validation failed: {message}") + print(" Please fix the validation errors before packaging.") + return None + print(f"✅ {message}\n") + + # Determine output location + skill_name = skill_path.name + if output_dir: + output_path = Path(output_dir).resolve() + output_path.mkdir(parents=True, exist_ok=True) + else: + output_path = Path.cwd() + + skill_filename = output_path / f"{skill_name}.skill" + + # Create the .skill file (zip format) + try: + with zipfile.ZipFile(skill_filename, "w", zipfile.ZIP_DEFLATED) as zipf: + # Walk through the skill directory + for file_path in skill_path.rglob("*"): + if file_path.is_file(): + # Calculate the relative path within the zip + arcname = file_path.relative_to(skill_path.parent) + zipf.write(file_path, arcname) + print(f" Added: {arcname}") + + print(f"\n✅ Successfully packaged skill to: {skill_filename}") + return skill_filename + + except Exception as e: + print(f"❌ Error creating .skill file: {e}") + return None + + +def main(): + if len(sys.argv) < 2: + print("Usage: python utils/package_skill.py [output-directory]") + print("\nExample:") + print(" python utils/package_skill.py skills/public/my-skill") + print(" python utils/package_skill.py skills/public/my-skill ./dist") + sys.exit(1) + + skill_path = sys.argv[1] + output_dir = sys.argv[2] if len(sys.argv) > 2 else None + + print(f"📦 Packaging skill: {skill_path}") + if output_dir: + print(f" Output directory: {output_dir}") + print() + + result = package_skill(skill_path, output_dir) + + if result: + sys.exit(0) + else: + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py b/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py new file mode 100644 index 000000000..4e99a7f9b --- /dev/null +++ b/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py @@ -0,0 +1,95 @@ +#!/usr/bin/env python3 +""" +Quick validation script for skills - minimal version +""" + +import re +import sys +from pathlib import Path + +import yaml + + +def validate_skill(skill_path): + """Basic validation of a skill""" + skill_path = Path(skill_path) + + skill_md = skill_path / "SKILL.md" + if not skill_md.exists(): + return False, "SKILL.md not found" + + content = skill_md.read_text() + if not content.startswith("---"): + return False, "No YAML frontmatter found" + + match = re.match(r"^---\n(.*?)\n---", content, re.DOTALL) + if not match: + return False, "Invalid frontmatter format" + + frontmatter_text = match.group(1) + + try: + frontmatter = yaml.safe_load(frontmatter_text) + if not isinstance(frontmatter, dict): + return False, "Frontmatter must be a YAML dictionary" + except yaml.YAMLError as e: + return False, f"Invalid YAML in frontmatter: {e}" + + allowed_properties = {"name", "description", "license", "allowed-tools", "metadata"} + + unexpected_keys = set(frontmatter.keys()) - allowed_properties + if unexpected_keys: + allowed = ", ".join(sorted(allowed_properties)) + unexpected = ", ".join(sorted(unexpected_keys)) + return ( + False, + f"Unexpected key(s) in SKILL.md frontmatter: {unexpected}. Allowed properties are: {allowed}", + ) + + if "name" not in frontmatter: + return False, "Missing 'name' in frontmatter" + if "description" not in frontmatter: + return False, "Missing 'description' in frontmatter" + + name = frontmatter.get("name", "") + if not isinstance(name, str): + return False, f"Name must be a string, got {type(name).__name__}" + name = name.strip() + if name: + if not re.match(r"^[a-z0-9-]+$", name): + return ( + False, + f"Name '{name}' should be hyphen-case (lowercase letters, digits, and hyphens only)", + ) + if name.startswith("-") or name.endswith("-") or "--" in name: + return ( + False, + f"Name '{name}' cannot start/end with hyphen or contain consecutive hyphens", + ) + if len(name) > 64: + return False, f"Name is too long ({len(name)} characters). Maximum is 64 characters." + + description = frontmatter.get("description", "") + if not isinstance(description, str): + return False, f"Description must be a string, got {type(description).__name__}" + description = description.strip() + if description: + if "<" in description or ">" in description: + return False, "Description cannot contain angle brackets (< or >)" + if len(description) > 1024: + return ( + False, + f"Description is too long ({len(description)} characters). Maximum is 1024 characters.", + ) + + return True, "Skill is valid!" + + +if __name__ == "__main__": + if len(sys.argv) != 2: + print("Usage: python quick_validate.py ") + sys.exit(1) + + valid, message = validate_skill(sys.argv[1]) + print(message) + sys.exit(0 if valid else 1) diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index 2596f3d76..32c5db843 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -3,7 +3,7 @@ use crate::git_info::resolve_root_git_project_for_trust; use crate::skills::model::SkillError; use crate::skills::model::SkillLoadOutcome; use crate::skills::model::SkillMetadata; -use crate::skills::public::public_cache_root_dir; +use crate::skills::system::system_cache_root_dir; use codex_protocol::protocol::SkillScope; use dunce::canonicalize as normalize_path; use serde::Deserialize; @@ -92,10 +92,10 @@ pub(crate) fn user_skills_root(codex_home: &Path) -> SkillRoot { } } -pub(crate) fn public_skills_root(codex_home: &Path) -> SkillRoot { +pub(crate) fn system_skills_root(codex_home: &Path) -> SkillRoot { SkillRoot { - path: public_cache_root_dir(codex_home), - scope: SkillScope::Public, + path: system_cache_root_dir(codex_home), + scope: SkillScope::System, } } @@ -139,9 +139,9 @@ fn skill_roots(config: &Config) -> Vec { } // Load order matters: we dedupe by name, keeping the first occurrence. - // This makes repo/user skills win over public skills. + // This makes repo/user skills win over system skills. roots.push(user_skills_root(&config.codex_home)); - roots.push(public_skills_root(&config.codex_home)); + roots.push(system_skills_root(&config.codex_home)); roots } @@ -195,7 +195,7 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil outcome.skills.push(skill); } Err(err) => { - if scope != SkillScope::Public { + if scope != SkillScope::System { outcome.errors.push(SkillError { path, message: err.to_string(), @@ -303,6 +303,20 @@ mod tests { write_skill_at(&codex_home.path().join("skills"), dir, name, description) } + fn write_system_skill( + codex_home: &TempDir, + dir: &str, + name: &str, + description: &str, + ) -> PathBuf { + write_skill_at( + &codex_home.path().join("skills/.system"), + dir, + name, + description, + ) + } + fn write_skill_at(root: &Path, dir: &str, name: &str, description: &str) -> PathBuf { let skill_dir = root.join(dir); fs::create_dir_all(&skill_dir).unwrap(); @@ -539,6 +553,25 @@ mod tests { assert_eq!(outcome.skills[0].scope, SkillScope::Repo); } + #[test] + fn loads_system_skills_with_lowest_priority() { + let codex_home = tempfile::tempdir().expect("tempdir"); + + write_system_skill(&codex_home, "system", "dupe-skill", "from system"); + write_skill(&codex_home, "user", "dupe-skill", "from user"); + + let cfg = make_config(&codex_home); + let outcome = load_skills(&cfg); + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); + assert_eq!(outcome.skills[0].description, "from user"); + assert_eq!(outcome.skills[0].scope, SkillScope::User); + } + #[test] fn repo_skills_search_does_not_escape_repo_root() { let codex_home = tempfile::tempdir().expect("tempdir"); @@ -643,16 +676,11 @@ mod tests { } #[test] - fn loads_skills_from_public_cache_when_present() { + fn loads_skills_from_system_cache_when_present() { let codex_home = tempfile::tempdir().expect("tempdir"); let work_dir = tempfile::tempdir().expect("tempdir"); - write_skill_at( - &codex_home.path().join("skills").join(".public"), - "public", - "public-skill", - "from public", - ); + write_system_skill(&codex_home, "system", "system-skill", "from system"); let mut cfg = make_config(&codex_home); cfg.cwd = work_dir.path().to_path_buf(); @@ -664,22 +692,17 @@ mod tests { outcome.errors ); assert_eq!(outcome.skills.len(), 1); - assert_eq!(outcome.skills[0].name, "public-skill"); - assert_eq!(outcome.skills[0].scope, SkillScope::Public); + assert_eq!(outcome.skills[0].name, "system-skill"); + assert_eq!(outcome.skills[0].scope, SkillScope::System); } #[test] - fn deduplicates_by_name_preferring_user_over_public() { + fn deduplicates_by_name_preferring_user_over_system() { let codex_home = tempfile::tempdir().expect("tempdir"); let work_dir = tempfile::tempdir().expect("tempdir"); write_skill(&codex_home, "user", "dupe-skill", "from user"); - write_skill_at( - &codex_home.path().join("skills").join(".public"), - "public", - "dupe-skill", - "from public", - ); + write_system_skill(&codex_home, "system", "dupe-skill", "from system"); let mut cfg = make_config(&codex_home); cfg.cwd = work_dir.path().to_path_buf(); @@ -696,7 +719,7 @@ mod tests { } #[test] - fn deduplicates_by_name_preferring_repo_over_public() { + fn deduplicates_by_name_preferring_repo_over_system() { let codex_home = tempfile::tempdir().expect("tempdir"); let repo_dir = tempfile::tempdir().expect("tempdir"); @@ -716,12 +739,7 @@ mod tests { "dupe-skill", "from repo", ); - write_skill_at( - &codex_home.path().join("skills").join(".public"), - "public", - "dupe-skill", - "from public", - ); + write_system_skill(&codex_home, "system", "dupe-skill", "from system"); let mut cfg = make_config(&codex_home); cfg.cwd = repo_dir.path().to_path_buf(); diff --git a/codex-rs/core/src/skills/manager.rs b/codex-rs/core/src/skills/manager.rs index f1c9f36a3..5ce174e4f 100644 --- a/codex-rs/core/src/skills/manager.rs +++ b/codex-rs/core/src/skills/manager.rs @@ -2,68 +2,35 @@ use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; use std::sync::RwLock; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering; use crate::skills::SkillLoadOutcome; use crate::skills::loader::load_skills_from_roots; -use crate::skills::loader::public_skills_root; use crate::skills::loader::repo_skills_root; +use crate::skills::loader::system_skills_root; use crate::skills::loader::user_skills_root; -use crate::skills::public::refresh_public_skills; -use tokio::sync::broadcast; - +use crate::skills::system::install_system_skills; pub struct SkillsManager { codex_home: PathBuf, cache_by_cwd: RwLock>, - attempted_public_refresh: AtomicBool, - skills_update_tx: broadcast::Sender<()>, } impl SkillsManager { pub fn new(codex_home: PathBuf) -> Self { - let (skills_update_tx, _skills_update_rx) = broadcast::channel(1); + if let Err(err) = install_system_skills(&codex_home) { + tracing::error!("failed to install system skills: {err}"); + } + Self { codex_home, cache_by_cwd: RwLock::new(HashMap::new()), - attempted_public_refresh: AtomicBool::new(false), - skills_update_tx, } } - pub(crate) fn subscribe_skills_update_notifications(&self) -> broadcast::Receiver<()> { - self.skills_update_tx.subscribe() - } - pub fn skills_for_cwd(&self, cwd: &Path) -> SkillLoadOutcome { self.skills_for_cwd_with_options(cwd, false) } - pub(crate) fn skills_for_cwd_with_options( - &self, - cwd: &Path, - force_reload: bool, - ) -> SkillLoadOutcome { - // Best-effort refresh: attempt at most once per manager instance. - if self - .attempted_public_refresh - .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) - .is_ok() - { - let codex_home = self.codex_home.clone(); - let skills_update_tx = self.skills_update_tx.clone(); - std::thread::spawn(move || match refresh_public_skills(&codex_home) { - Ok(outcome) => { - if outcome.updated() { - let _ = skills_update_tx.send(()); - } - } - Err(err) => { - tracing::error!("failed to refresh public skills: {err}"); - } - }); - } - + pub fn skills_for_cwd_with_options(&self, cwd: &Path, force_reload: bool) -> SkillLoadOutcome { let cached = match self.cache_by_cwd.read() { Ok(cache) => cache.get(cwd).cloned(), Err(err) => err.into_inner().get(cwd).cloned(), @@ -77,7 +44,7 @@ impl SkillsManager { roots.push(repo_root); } roots.push(user_skills_root(&self.codex_home)); - roots.push(public_skills_root(&self.codex_home)); + roots.push(system_skills_root(&self.codex_home)); let outcome = load_skills_from_roots(roots); match self.cache_by_cwd.write() { Ok(mut cache) => { diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index 4872b6a2f..cf7c18050 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -2,8 +2,8 @@ pub mod injection; pub mod loader; pub mod manager; pub mod model; -pub mod public; pub mod render; +pub mod system; pub(crate) use injection::SkillInjections; pub(crate) use injection::build_skill_injections; diff --git a/codex-rs/core/src/skills/public.rs b/codex-rs/core/src/skills/public.rs deleted file mode 100644 index 8909d6a55..000000000 --- a/codex-rs/core/src/skills/public.rs +++ /dev/null @@ -1,397 +0,0 @@ -use std::fs; -use std::path::Path; -use std::path::PathBuf; -use std::process::ExitStatus; -use std::time::SystemTime; -use std::time::UNIX_EPOCH; - -use thiserror::Error; - -const PUBLIC_SKILLS_REPO_URL: &str = "https://github.com/openai/skills.git"; -const PUBLIC_SKILLS_DIR_NAME: &str = ".public"; -const SKILLS_DIR_NAME: &str = "skills"; - -struct TempDirCleanup { - path: PathBuf, - // Disable Drop cleanup after explicit cleanup to avoid double-delete. - active: bool, -} - -impl TempDirCleanup { - fn new(path: PathBuf) -> Self { - Self { path, active: true } - } - - fn cleanup(&mut self) -> Result<(), PublicSkillsError> { - if self.active && self.path.exists() { - fs::remove_dir_all(&self.path) - .map_err(|source| PublicSkillsError::io("remove public skills tmp dir", source))?; - } - self.active = false; - Ok(()) - } -} - -impl Drop for TempDirCleanup { - fn drop(&mut self) { - if self.active && self.path.exists() { - let _ = fs::remove_dir_all(&self.path); - } - } -} - -pub(crate) fn public_cache_root_dir(codex_home: &Path) -> PathBuf { - codex_home - .join(SKILLS_DIR_NAME) - .join(PUBLIC_SKILLS_DIR_NAME) -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum PublicSkillsRefreshOutcome { - Skipped, - Updated, -} - -impl PublicSkillsRefreshOutcome { - pub(crate) fn updated(self) -> bool { - matches!(self, Self::Updated) - } -} - -pub(crate) fn refresh_public_skills( - codex_home: &Path, -) -> Result { - // Keep tests deterministic and offline-safe. Tests that want to exercise the - // refresh behavior should call `refresh_public_skills_from_repo_url`. - if cfg!(test) { - return Ok(PublicSkillsRefreshOutcome::Skipped); - } - refresh_public_skills_inner(codex_home, PUBLIC_SKILLS_REPO_URL) -} - -#[cfg(test)] -pub(crate) fn refresh_public_skills_from_repo_url( - codex_home: &Path, - repo_url: &str, -) -> Result { - refresh_public_skills_inner(codex_home, repo_url) -} - -fn refresh_public_skills_inner( - codex_home: &Path, - repo_url: &str, -) -> Result { - // Best-effort refresh: clone the repo to a temp dir, stage its `skills/`, then atomically swap - // the staged directory into the public cache. - let skills_root_dir = codex_home.join(SKILLS_DIR_NAME); - fs::create_dir_all(&skills_root_dir) - .map_err(|source| PublicSkillsError::io("create skills root dir", source))?; - - let dest_public = public_cache_root_dir(codex_home); - - let tmp_dir = skills_root_dir.join(format!(".public-tmp-{}", rand_suffix())); - if tmp_dir.exists() { - fs::remove_dir_all(&tmp_dir).map_err(|source| { - PublicSkillsError::io("remove existing public skills tmp dir", source) - })?; - } - fs::create_dir_all(&tmp_dir) - .map_err(|source| PublicSkillsError::io("create public skills tmp dir", source))?; - let mut tmp_dir_cleanup = TempDirCleanup::new(tmp_dir.clone()); - - let checkout_dir = tmp_dir.join("checkout"); - clone_repo(repo_url, &checkout_dir)?; - - let src_skills = checkout_dir.join(SKILLS_DIR_NAME); - let src_skills_metadata = fs::symlink_metadata(&src_skills) - .map_err(|source| PublicSkillsError::io("read skills dir metadata", source))?; - let src_skills_type = src_skills_metadata.file_type(); - if src_skills_type.is_symlink() || !src_skills_type.is_dir() { - return Err(PublicSkillsError::RepoMissingSkillsDir { - skills_dir_name: SKILLS_DIR_NAME, - }); - } - - let staged_public = tmp_dir.join(PUBLIC_SKILLS_DIR_NAME); - stage_skills_dir(&src_skills, &staged_public)?; - - atomic_swap_dir(&staged_public, &dest_public, &skills_root_dir)?; - - tmp_dir_cleanup.cleanup()?; - Ok(PublicSkillsRefreshOutcome::Updated) -} - -fn stage_skills_dir(src: &Path, staged: &Path) -> Result<(), PublicSkillsError> { - fs::rename(src, staged).map_err(|source| PublicSkillsError::io("stage skills dir", source))?; - - prune_symlinks_and_special_files(staged)?; - Ok(()) -} - -fn prune_symlinks_and_special_files(root: &Path) -> Result<(), PublicSkillsError> { - let mut stack: Vec = vec![root.to_path_buf()]; - while let Some(dir) = stack.pop() { - for entry in fs::read_dir(&dir) - .map_err(|source| PublicSkillsError::io("read staged skills dir", source))? - { - let entry = entry - .map_err(|source| PublicSkillsError::io("read staged skills dir entry", source))?; - let file_type = entry - .file_type() - .map_err(|source| PublicSkillsError::io("read staged skills entry type", source))?; - let path = entry.path(); - - if file_type.is_symlink() { - fs::remove_file(&path).map_err(|source| { - PublicSkillsError::io("remove symlink from staged skills", source) - })?; - continue; - } - - if file_type.is_dir() { - stack.push(path); - continue; - } - - if file_type.is_file() { - continue; - } - - fs::remove_file(&path).map_err(|source| { - PublicSkillsError::io("remove special file from staged skills", source) - })?; - } - } - - Ok(()) -} - -fn clone_repo(repo_url: &str, checkout_dir: &Path) -> Result<(), PublicSkillsError> { - let out = std::process::Command::new("git") - .env("GIT_TERMINAL_PROMPT", "0") - .env("GIT_ASKPASS", "true") - .arg("clone") - .arg("--depth") - .arg("1") - .arg(repo_url) - .arg(checkout_dir) - .stdin(std::process::Stdio::null()) - .output() - .map_err(|source| PublicSkillsError::io("spawn `git clone`", source))?; - if !out.status.success() { - let stderr = String::from_utf8_lossy(&out.stderr); - let stderr = stderr.trim(); - return if stderr.is_empty() { - Err(PublicSkillsError::GitCloneFailed { status: out.status }) - } else { - Err(PublicSkillsError::GitCloneFailedWithStderr { - status: out.status, - stderr: stderr.to_owned(), - }) - }; - } - Ok(()) -} - -fn atomic_swap_dir(staged: &Path, dest: &Path, parent: &Path) -> Result<(), PublicSkillsError> { - if let Some(dest_parent) = dest.parent() { - fs::create_dir_all(dest_parent) - .map_err(|source| PublicSkillsError::io("create public skills dest parent", source))?; - } - - let backup_base = dest - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or("skills"); - let backup = parent.join(format!("{backup_base}.old-{}", rand_suffix())); - if backup.exists() { - fs::remove_dir_all(&backup) - .map_err(|source| PublicSkillsError::io("remove old public skills backup", source))?; - } - - if dest.exists() { - fs::rename(dest, &backup) - .map_err(|source| PublicSkillsError::io("rename public skills to backup", source))?; - } - - if let Err(err) = fs::rename(staged, dest) { - if backup.exists() { - let _ = fs::rename(&backup, dest); - } - return Err(PublicSkillsError::io( - "rename staged public skills into place", - err, - )); - } - - if backup.exists() { - fs::remove_dir_all(&backup) - .map_err(|source| PublicSkillsError::io("remove public skills backup", source))?; - } - - Ok(()) -} - -fn rand_suffix() -> String { - let pid = std::process::id(); - let nanos = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos(); - format!("{pid:x}-{nanos:x}") -} - -#[derive(Debug, Error)] -pub(crate) enum PublicSkillsError { - #[error("io error while {action}: {source}")] - Io { - action: &'static str, - #[source] - source: std::io::Error, - }, - - #[error("repo did not contain a `{skills_dir_name}` directory")] - RepoMissingSkillsDir { skills_dir_name: &'static str }, - - #[error("`git clone` failed with status {status}")] - GitCloneFailed { status: ExitStatus }, - - #[error("`git clone` failed with status {status}: {stderr}")] - GitCloneFailedWithStderr { status: ExitStatus, stderr: String }, -} - -impl PublicSkillsError { - fn io(action: &'static str, source: std::io::Error) -> Self { - Self::Io { action, source } - } -} - -#[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - use tempfile::TempDir; - - fn write_public_skill(repo_dir: &TempDir, name: &str, description: &str) { - let skills_dir = repo_dir.path().join("skills").join(name); - fs::create_dir_all(&skills_dir).unwrap(); - let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n"); - fs::write(skills_dir.join("SKILL.md"), content).unwrap(); - } - - fn git(repo_dir: &TempDir, args: &[&str]) { - let status = std::process::Command::new("git") - .args([ - "-c", - "user.name=codex-test", - "-c", - "user.email=codex-test@example.com", - ]) - .args(args) - .current_dir(repo_dir.path()) - .status() - .unwrap(); - assert!(status.success(), "git command failed: {args:?}"); - } - - #[tokio::test] - async fn refresh_copies_skills_subdir_into_public_cache() { - let codex_home = tempfile::tempdir().unwrap(); - let repo_dir = tempfile::tempdir().unwrap(); - git(&repo_dir, &["init"]); - write_public_skill(&repo_dir, "demo", "from repo"); - git(&repo_dir, &["add", "."]); - git(&repo_dir, &["commit", "-m", "init"]); - - refresh_public_skills_from_repo_url(codex_home.path(), repo_dir.path().to_str().unwrap()) - .unwrap(); - - let path = public_cache_root_dir(codex_home.path()) - .join("demo") - .join("SKILL.md"); - let contents = fs::read_to_string(path).unwrap(); - assert!(contents.contains("name: demo")); - assert!(contents.contains("description: from repo")); - } - - #[tokio::test] - async fn refresh_overwrites_existing_public_cache() { - let codex_home = tempfile::tempdir().unwrap(); - let repo_dir = tempfile::tempdir().unwrap(); - git(&repo_dir, &["init"]); - write_public_skill(&repo_dir, "demo", "v1"); - git(&repo_dir, &["add", "."]); - git(&repo_dir, &["commit", "-m", "v1"]); - - refresh_public_skills_from_repo_url(codex_home.path(), repo_dir.path().to_str().unwrap()) - .unwrap(); - - write_public_skill(&repo_dir, "demo", "v2"); - git(&repo_dir, &["add", "."]); - git(&repo_dir, &["commit", "-m", "v2"]); - - refresh_public_skills_from_repo_url(codex_home.path(), repo_dir.path().to_str().unwrap()) - .unwrap(); - - let path = public_cache_root_dir(codex_home.path()) - .join("demo") - .join("SKILL.md"); - let contents = fs::read_to_string(path).unwrap(); - assert_eq!(contents.matches("description:").count(), 1); - assert!(contents.contains("description: v2")); - } - - #[cfg(unix)] - #[tokio::test] - async fn refresh_prunes_symlinks_inside_skills_dir() { - use std::os::unix::fs::symlink; - - let codex_home = tempfile::tempdir().unwrap(); - let repo_dir = tempfile::tempdir().unwrap(); - git(&repo_dir, &["init"]); - write_public_skill(&repo_dir, "demo", "from repo"); - - let demo_dir = repo_dir.path().join("skills").join("demo"); - symlink("SKILL.md", demo_dir.join("link-to-skill")).unwrap(); - git(&repo_dir, &["add", "."]); - git(&repo_dir, &["commit", "-m", "init"]); - - refresh_public_skills_from_repo_url(codex_home.path(), repo_dir.path().to_str().unwrap()) - .unwrap(); - - assert!( - !public_cache_root_dir(codex_home.path()) - .join("demo") - .join("link-to-skill") - .exists() - ); - } - - #[cfg(unix)] - #[tokio::test] - async fn refresh_rejects_symlinked_skills_dir() { - use std::os::unix::fs::symlink; - - let codex_home = tempfile::tempdir().unwrap(); - let repo_dir = tempfile::tempdir().unwrap(); - git(&repo_dir, &["init"]); - - let skills_target = repo_dir.path().join("skills-target"); - fs::create_dir_all(skills_target.join("demo")).unwrap(); - fs::write( - skills_target.join("demo").join("SKILL.md"), - "---\nname: demo\ndescription: from repo\n---\n", - ) - .unwrap(); - symlink("skills-target", repo_dir.path().join("skills")).unwrap(); - git(&repo_dir, &["add", "."]); - git(&repo_dir, &["commit", "-m", "init"]); - - let err = refresh_public_skills_from_repo_url( - codex_home.path(), - repo_dir.path().to_str().unwrap(), - ) - .unwrap_err(); - assert!(err.to_string().contains("repo did not contain")); - } -} diff --git a/codex-rs/core/src/skills/system.rs b/codex-rs/core/src/skills/system.rs new file mode 100644 index 000000000..978438d9d --- /dev/null +++ b/codex-rs/core/src/skills/system.rs @@ -0,0 +1,163 @@ +use codex_utils_absolute_path::AbsolutePathBuf; +use include_dir::Dir; +use std::collections::hash_map::DefaultHasher; +use std::fs; +use std::hash::Hash; +use std::hash::Hasher; +use std::path::Path; +use std::path::PathBuf; + +use thiserror::Error; + +const SYSTEM_SKILLS_DIR: Dir = + include_dir::include_dir!("$CARGO_MANIFEST_DIR/src/skills/assets/samples"); + +const SYSTEM_SKILLS_DIR_NAME: &str = ".system"; +const SKILLS_DIR_NAME: &str = "skills"; +const SYSTEM_SKILLS_MARKER_FILENAME: &str = ".codex-system-skills.marker"; + +/// Returns the on-disk cache location for embedded system skills. +/// +/// This is typically located at `CODEX_HOME/skills/.system`. +pub(crate) fn system_cache_root_dir(codex_home: &Path) -> PathBuf { + AbsolutePathBuf::try_from(codex_home) + .and_then(|codex_home| system_cache_root_dir_abs(&codex_home)) + .map(AbsolutePathBuf::into_path_buf) + .unwrap_or_else(|_| { + codex_home + .join(SKILLS_DIR_NAME) + .join(SYSTEM_SKILLS_DIR_NAME) + }) +} + +fn system_cache_root_dir_abs(codex_home: &AbsolutePathBuf) -> std::io::Result { + codex_home + .join(SKILLS_DIR_NAME)? + .join(SYSTEM_SKILLS_DIR_NAME) +} + +/// Installs embedded system skills into `CODEX_HOME/skills/.system`. +/// +/// Clears any existing system skills directory first and then writes the embedded +/// skills directory into place. +/// +/// To avoid doing unnecessary work on every startup, a marker file is written +/// with a fingerprint of the embedded directory. When the marker matches, the +/// install is skipped. +pub(crate) fn install_system_skills(codex_home: &Path) -> Result<(), SystemSkillsError> { + let codex_home = AbsolutePathBuf::try_from(codex_home) + .map_err(|source| SystemSkillsError::io("normalize codex home dir", source))?; + let skills_root_dir = codex_home + .join(SKILLS_DIR_NAME) + .map_err(|source| SystemSkillsError::io("resolve skills root dir", source))?; + fs::create_dir_all(skills_root_dir.as_path()) + .map_err(|source| SystemSkillsError::io("create skills root dir", source))?; + + let dest_system = system_cache_root_dir_abs(&codex_home) + .map_err(|source| SystemSkillsError::io("resolve system skills cache root dir", source))?; + + let marker_path = dest_system + .join(SYSTEM_SKILLS_MARKER_FILENAME) + .map_err(|source| SystemSkillsError::io("resolve system skills marker path", source))?; + let expected_fingerprint = embedded_system_skills_fingerprint(); + if dest_system.as_path().is_dir() + && read_marker(&marker_path).is_ok_and(|marker| marker == expected_fingerprint) + { + return Ok(()); + } + + if dest_system.as_path().exists() { + fs::remove_dir_all(dest_system.as_path()) + .map_err(|source| SystemSkillsError::io("remove existing system skills dir", source))?; + } + + write_embedded_dir(&SYSTEM_SKILLS_DIR, &dest_system)?; + fs::write(marker_path.as_path(), format!("{expected_fingerprint}\n")) + .map_err(|source| SystemSkillsError::io("write system skills marker", source))?; + Ok(()) +} + +fn read_marker(path: &AbsolutePathBuf) -> Result { + Ok(fs::read_to_string(path.as_path()) + .map_err(|source| SystemSkillsError::io("read system skills marker", source))? + .trim() + .to_string()) +} + +fn embedded_system_skills_fingerprint() -> String { + let mut items: Vec<(String, Option)> = SYSTEM_SKILLS_DIR + .entries() + .iter() + .map(|entry| match entry { + include_dir::DirEntry::Dir(dir) => (dir.path().to_string_lossy().to_string(), None), + include_dir::DirEntry::File(file) => { + let mut file_hasher = DefaultHasher::new(); + file.contents().hash(&mut file_hasher); + ( + file.path().to_string_lossy().to_string(), + Some(file_hasher.finish()), + ) + } + }) + .collect(); + items.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); + + let mut hasher = DefaultHasher::new(); + for (path, contents_hash) in items { + path.hash(&mut hasher); + contents_hash.hash(&mut hasher); + } + format!("{:x}", hasher.finish()) +} + +/// Writes the embedded `include_dir::Dir` to disk under `dest`. +/// +/// Preserves the embedded directory structure. +fn write_embedded_dir(dir: &Dir<'_>, dest: &AbsolutePathBuf) -> Result<(), SystemSkillsError> { + fs::create_dir_all(dest.as_path()) + .map_err(|source| SystemSkillsError::io("create system skills dir", source))?; + + for entry in dir.entries() { + match entry { + include_dir::DirEntry::Dir(subdir) => { + let subdir_dest = dest.join(subdir.path()).map_err(|source| { + SystemSkillsError::io("resolve system skills subdir", source) + })?; + fs::create_dir_all(subdir_dest.as_path()).map_err(|source| { + SystemSkillsError::io("create system skills subdir", source) + })?; + write_embedded_dir(subdir, dest)?; + } + include_dir::DirEntry::File(file) => { + let path = dest.join(file.path()).map_err(|source| { + SystemSkillsError::io("resolve system skills file", source) + })?; + if let Some(parent) = path.as_path().parent() { + fs::create_dir_all(parent).map_err(|source| { + SystemSkillsError::io("create system skills file parent", source) + })?; + } + fs::write(path.as_path(), file.contents()) + .map_err(|source| SystemSkillsError::io("write system skill file", source))?; + } + } + } + + Ok(()) +} + +#[derive(Debug, Error)] +pub(crate) enum SystemSkillsError { + #[error("io error while {action}: {source}")] + Io { + action: &'static str, + #[source] + source: std::io::Error, + }, +} + +impl SystemSkillsError { + fn io(action: &'static str, source: std::io::Error) -> Self { + Self::Io { action, source } + } +} diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 42a06d3f3..1a1456f1a 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -15,7 +15,6 @@ use codex_core::WireApi; use codex_core::auth::AuthCredentialsStoreMode; use codex_core::built_in_model_providers; use codex_core::error::CodexErr; -use codex_core::features::Feature; use codex_core::openai_models::models_manager::ModelsManager; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; @@ -651,7 +650,7 @@ async fn includes_user_instructions_message_in_request() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn skills_append_to_instructions_when_feature_enabled() { +async fn skills_append_to_instructions() { skip_if_no_network!(); let server = MockServer::start().await; @@ -673,7 +672,6 @@ async fn skills_append_to_instructions_when_feature_enabled() { let mut config = load_default_config_for_test(&codex_home); config.model_provider = model_provider; - config.features.enable(Feature::Skills); config.cwd = codex_home.path().to_path_buf(); let conversation_manager = ConversationManager::with_models_provider_and_home( diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index f223f4d10..dd8e4ca2c 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -589,7 +589,36 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { .body_json::() .unwrap(); let input = body.get("input").and_then(|v| v.as_array()).unwrap(); - let environment_message = input[0]["content"][0]["text"].as_str().unwrap(); + + fn normalize_inputs(values: &[serde_json::Value]) -> Vec { + values + .iter() + .filter(|value| { + if value + .get("type") + .and_then(|ty| ty.as_str()) + .is_some_and(|ty| ty == "function_call_output") + { + return false; + } + + let text = value + .get("content") + .and_then(|content| content.as_array()) + .and_then(|content| content.first()) + .and_then(|item| item.get("text")) + .and_then(|text| text.as_str()); + + // Ignore the cached UI prefix (project docs + skills) since it is not relevant to + // compaction behavior and can change as bundled skills evolve. + !text.is_some_and(|text| text.starts_with("# AGENTS.md instructions for ")) + }) + .cloned() + .collect() + } + + let initial_input = normalize_inputs(input); + let environment_message = initial_input[0]["content"][0]["text"].as_str().unwrap(); // test 1: after compaction, we should have one environment message, one user message, and one user message with summary prefix let compaction_indices = [2, 4, 6]; @@ -603,6 +632,7 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { .body_json::() .unwrap(); let input = body.get("input").and_then(|v| v.as_array()).unwrap(); + let input = normalize_inputs(input); assert_eq!(input.len(), 3); let environment_message = input[0]["content"][0]["text"].as_str().unwrap(); let user_message_received = input[1]["content"][0]["text"].as_str().unwrap(); @@ -962,20 +992,6 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { ] ]); - // ignore local shell calls output because it differs from OS to another and it's out of the scope of this test. - fn normalize_inputs(values: &[serde_json::Value]) -> Vec { - values - .iter() - .filter(|value| { - value - .get("type") - .and_then(|ty| ty.as_str()) - .is_none_or(|ty| ty != "function_call_output") - }) - .cloned() - .collect() - } - for (i, request) in requests_payloads.iter().enumerate() { let body = request.body_json::().unwrap(); let input = body.get("input").and_then(|v| v.as_array()).unwrap(); diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index a562aa5a8..b0b58b8d8 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -251,49 +251,36 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests .await?; wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + let body1 = req1.single_request().body_json(); + let input1 = body1["input"].as_array().expect("input array"); + assert_eq!(input1.len(), 3, "expected cached prefix + env + user msg"); + + let ui_text = input1[0]["content"][0]["text"] + .as_str() + .expect("ui message text"); + assert!( + ui_text.contains("be consistent and helpful"), + "expected user instructions in UI message: {ui_text}" + ); + let shell = default_user_shell(); let cwd_str = config.cwd.to_string_lossy(); let expected_env_text = default_env_context_str(&cwd_str, &shell); - let expected_ui_text = format!( - "# AGENTS.md instructions for {cwd_str}\n\n\nbe consistent and helpful\n" - ); - - let expected_env_msg = serde_json::json!({ - "type": "message", - "role": "user", - "content": [ { "type": "input_text", "text": expected_env_text } ] - }); - let expected_ui_msg = serde_json::json!({ - "type": "message", - "role": "user", - "content": [ { "type": "input_text", "text": expected_ui_text } ] - }); - - let expected_user_message_1 = serde_json::json!({ - "type": "message", - "role": "user", - "content": [ { "type": "input_text", "text": "hello 1" } ] - }); - let body1 = req1.single_request().body_json(); assert_eq!( - body1["input"], - serde_json::json!([expected_ui_msg, expected_env_msg, expected_user_message_1]) + input1[1], + text_user_input(expected_env_text), + "expected environment context after UI message" ); + assert_eq!(input1[2], text_user_input("hello 1".to_string())); - let expected_user_message_2 = serde_json::json!({ - "type": "message", - "role": "user", - "content": [ { "type": "input_text", "text": "hello 2" } ] - }); let body2 = req2.single_request().body_json(); - let expected_body2 = serde_json::json!( - [ - body1["input"].as_array().unwrap().as_slice(), - [expected_user_message_2].as_slice(), - ] - .concat() + let input2 = body2["input"].as_array().expect("input array"); + assert_eq!( + &input2[..input1.len()], + input1.as_slice(), + "expected cached prefix to be reused" ); - assert_eq!(body2["input"], expected_body2); + assert_eq!(input2[input1.len()], text_user_input("hello 2".to_string())); Ok(()) } @@ -439,17 +426,21 @@ async fn override_before_first_turn_emits_environment_context() -> anyhow::Resul "expected at least environment context and user message" ); - let env_msg = &input[1]; - let env_text = env_msg["content"][0]["text"] - .as_str() - .expect("environment context text"); + let env_texts: Vec<&str> = input + .iter() + .filter_map(|msg| { + msg["content"] + .as_array() + .and_then(|content| content.first()) + .and_then(|item| item["text"].as_str()) + }) + .filter(|text| text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG)) + .collect(); assert!( - env_text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG), - "second entry should be environment context, got: {env_text}" - ); - assert!( - env_text.contains("never"), - "environment context should reflect overridden approval policy: {env_text}" + env_texts + .iter() + .any(|text| text.contains("never")), + "environment context should reflect overridden approval policy: {env_texts:?}" ); let env_count = input @@ -474,11 +465,19 @@ async fn override_before_first_turn_emits_environment_context() -> anyhow::Resul "environment context should appear exactly twice, found {env_count}" ); - let user_msg = &input[2]; - let user_text = user_msg["content"][0]["text"] - .as_str() - .expect("user message text"); - assert_eq!(user_text, "first message"); + let user_texts: Vec<&str> = input + .iter() + .filter_map(|msg| { + msg["content"] + .as_array() + .and_then(|content| content.first()) + .and_then(|item| item["text"].as_str()) + }) + .collect(); + assert!( + user_texts.contains(&"first message"), + "expected user message text, got {user_texts:?}" + ); Ok(()) } @@ -646,12 +645,10 @@ async fn send_user_turn_with_no_changes_does_not_send_environment_context() -> a let body1 = req1.single_request().body_json(); let body2 = req2.single_request().body_json(); + let expected_ui_msg = body1["input"][0].clone(); + let shell = default_user_shell(); let default_cwd_lossy = default_cwd.to_string_lossy(); - let expected_ui_text = format!( - "# AGENTS.md instructions for {default_cwd_lossy}\n\n\nbe consistent and helpful\n" - ); - let expected_ui_msg = text_user_input(expected_ui_text); let expected_env_msg_1 = text_user_input(default_env_context_str(&default_cwd_lossy, &shell)); let expected_user_message_1 = text_user_input("hello 1".to_string()); @@ -738,16 +735,9 @@ async fn send_user_turn_with_changes_sends_environment_context() -> anyhow::Resu let body1 = req1.single_request().body_json(); let body2 = req2.single_request().body_json(); + let expected_ui_msg = body1["input"][0].clone(); + let shell = default_user_shell(); - let expected_ui_text = format!( - "# AGENTS.md instructions for {}\n\n\nbe consistent and helpful\n", - default_cwd.to_string_lossy() - ); - let expected_ui_msg = serde_json::json!({ - "type": "message", - "role": "user", - "content": [ { "type": "input_text", "text": expected_ui_text } ] - }); let expected_env_text_1 = default_env_context_str(&default_cwd.to_string_lossy(), &shell); let expected_env_msg_1 = text_user_input(expected_env_text_1); let expected_user_message_1 = text_user_input("hello 1".to_string()); diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index 1fd644f8e..4597c0f19 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -553,31 +553,28 @@ async fn review_input_isolated_from_parent_history() { .expect("expected POST request to /responses"); let body = request.body_json::().unwrap(); let input = body["input"].as_array().expect("input array"); - assert_eq!( - input.len(), - 2, - "expected environment context and review prompt" + assert!( + input.len() >= 2, + "expected at least environment context and review prompt" ); - let env_msg = &input[0]; - assert_eq!(env_msg["type"].as_str().unwrap(), "message"); - assert_eq!(env_msg["role"].as_str().unwrap(), "user"); - let env_text = env_msg["content"][0]["text"].as_str().expect("env text"); - assert!( - env_text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG), - "environment context must be the first item" - ); + let env_text = input + .iter() + .filter_map(|msg| msg["content"][0]["text"].as_str()) + .find(|text| text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG)) + .expect("env text"); assert!( env_text.contains(""), "environment context should include cwd" ); - let review_msg = &input[1]; - assert_eq!(review_msg["type"].as_str().unwrap(), "message"); - assert_eq!(review_msg["role"].as_str().unwrap(), "user"); + let review_text = input + .iter() + .filter_map(|msg| msg["content"][0]["text"].as_str()) + .find(|text| *text == review_prompt) + .expect("review prompt text"); assert_eq!( - review_msg["content"][0]["text"].as_str().unwrap(), - review_prompt, + review_text, review_prompt, "user message should only contain the raw review prompt" ); diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index 2e3a8f5f1..cee44f0d9 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -311,10 +311,9 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { #[cfg_attr(target_os = "windows", ignore)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_snapshot_deleted_after_shutdown_with_skills_enabled() -> Result<()> { +async fn shell_snapshot_deleted_after_shutdown_with_skills() -> Result<()> { let builder = test_codex().with_config(|config| { config.features.enable(Feature::ShellSnapshot); - config.features.enable(Feature::Skills); }); let harness = TestCodexHarness::with_builder(builder).await?; let home = harness.test().home.clone(); diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index b23b3fd5b..c97dd29bb 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -2,7 +2,6 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use anyhow::Result; -use codex_core::features::Feature; use codex_core::protocol::AskForApproval; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; @@ -27,33 +26,15 @@ fn write_skill(home: &Path, name: &str, description: &str, body: &str) -> std::p path } -fn write_public_skill( - home: &Path, - name: &str, - description: &str, - body: &str, -) -> std::path::PathBuf { - let skill_dir = home.join("skills").join(".public").join(name); - fs::create_dir_all(&skill_dir).unwrap(); - let contents = format!("---\nname: {name}\ndescription: {description}\n---\n\n{body}\n"); - let path = skill_dir.join("SKILL.md"); - fs::write(&path, contents).unwrap(); - path -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn user_turn_includes_skill_instructions() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; let skill_body = "skill body"; - let mut builder = test_codex() - .with_config(|cfg| { - cfg.features.enable(Feature::Skills); - }) - .with_pre_build_hook(|home| { - write_skill(home, "demo", "demo skill", skill_body); - }); + let mut builder = test_codex().with_pre_build_hook(|home| { + write_skill(home, "demo", "demo skill", skill_body); + }); let test = builder.build(&server).await?; let skill_path = test.codex_home_path().join("skills/demo/SKILL.md"); @@ -117,15 +98,11 @@ async fn skill_load_errors_surface_in_session_configured() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut builder = test_codex() - .with_config(|cfg| { - cfg.features.enable(Feature::Skills); - }) - .with_pre_build_hook(|home| { - let skill_dir = home.join("skills").join("broken"); - fs::create_dir_all(&skill_dir).unwrap(); - fs::write(skill_dir.join("SKILL.md"), "not yaml").unwrap(); - }); + let mut builder = test_codex().with_pre_build_hook(|home| { + let skill_dir = home.join("skills").join("broken"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write(skill_dir.join("SKILL.md"), "not yaml").unwrap(); + }); let test = builder.build(&server).await?; test.codex @@ -169,19 +146,30 @@ async fn skill_load_errors_surface_in_session_configured() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn list_skills_includes_public_cache_entries() -> Result<()> { +async fn list_skills_includes_system_cache_entries() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut builder = test_codex() - .with_config(|cfg| { - cfg.features.enable(Feature::Skills); - }) - .with_pre_build_hook(|home| { - write_public_skill(home, "public-demo", "public skill", "public body"); - }); + let mut builder = test_codex().with_pre_build_hook(|home| { + let system_skill_path = home.join("skills/.system/plan/SKILL.md"); + assert!( + !system_skill_path.exists(), + "expected embedded system skills not yet installed, but {system_skill_path:?} exists" + ); + }); let test = builder.build(&server).await?; + let system_skill_path = test.codex_home_path().join("skills/.system/plan/SKILL.md"); + assert!( + system_skill_path.exists(), + "expected embedded system skills installed to {system_skill_path:?}" + ); + let system_skill_contents = fs::read_to_string(&system_skill_path)?; + assert!( + system_skill_contents.contains("name: plan"), + "expected embedded system skill file, got:\n{system_skill_contents}" + ); + test.codex .submit(Op::ListSkills { cwds: Vec::new(), @@ -205,12 +193,12 @@ async fn list_skills_includes_public_cache_entries() -> Result<()> { let skill = skills .iter() - .find(|skill| skill.name == "public-demo") - .expect("expected public skill to be present"); - assert_eq!(skill.scope, codex_protocol::protocol::SkillScope::Public); + .find(|skill| skill.name == "plan") + .expect("expected system skill to be present"); + assert_eq!(skill.scope, codex_protocol::protocol::SkillScope::System); let path_str = skill.path.to_string_lossy().replace('\\', "/"); assert!( - path_str.ends_with("/skills/.public/public-demo/SKILL.md"), + path_str.ends_with("/skills/.system/plan/SKILL.md"), "unexpected skill path: {path_str}" ); diff --git a/codex-rs/docs/protocol_v1.md b/codex-rs/docs/protocol_v1.md index 682a38bfc..805abb0ea 100644 --- a/codex-rs/docs/protocol_v1.md +++ b/codex-rs/docs/protocol_v1.md @@ -77,7 +77,6 @@ For complete documentation of the `Op` and `EventMsg` variants, refer to [protoc - `EventMsg::Warning` – A non-fatal warning that the client should surface to the user - `EventMsg::TurnComplete` – Contains a `response_id` bookmark for last `response_id` executed by the task. This can be used to continue the task at a later point in time, perhaps with additional user input. - `EventMsg::ListSkillsResponse` – Response payload with per-cwd skill entries (`cwd`, `skills`, `errors`) - - `EventMsg::SkillsUpdateAvailable` – Notification that skills may have changed and clients may want to reload The `response_id` returned from each task matches the OpenAI `response_id` stored in the API's `/responses` endpoint. It can be stored and used in future `Sessions` to resume threads of work. diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2fdb022b8..b3165acbe 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1690,7 +1690,7 @@ pub struct ListSkillsResponseEvent { pub enum SkillScope { User, Repo, - Public, + System, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]