From 097ef52a2a3cc6f2799ea5e11787c16eeea66fd2 Mon Sep 17 00:00:00 2001 From: Chris Wing Date: Thu, 25 Jun 2026 19:56:31 -0700 Subject: [PATCH 1/2] feat: agent skill evaluation infrastructure Adds skills as a run-level evaluation variable, decoupled from the dataset. Skills are specified via skills.path on ng_collect_rollouts and applied to a fixed, skill-agnostic dataset at rollout time. Each rollout result is stamped with a content-hashed skills_ref for provenance and variant comparison. - nemo_gym/skills.py: load_skill_directory, hash_skill_dir, stage_skills, and SkillsConfig/SkillsRef models (version parsed from spec metadata.version, SKILL.md decoded utf-8-sig to tolerate a BOM). - rollout_collection.py: optional skills config; resolve once at startup, stamp skills_ref onto each row and propagate to results (survives resume_from_cache). - claude_code_agent: reads skills_ref.path and stages it into the per-request CLAUDE_CONFIG_DIR/skills/, forcing --bare off so native discovery loads it; cleans up a partially-created config dir if staging fails. - docs: agent-server/agent-skills reference page. Signed-off-by: Chris Wing --- .../pages/agent-server/agent-skills.mdx | 105 +++++++++ nemo_gym/global_config.py | 1 + nemo_gym/rollout_collection.py | 24 +++ nemo_gym/skills.py | 201 ++++++++++++++++++ .../claude_code_agent/README.md | 49 +++++ responses_api_agents/claude_code_agent/app.py | 66 ++++-- .../claude_code_agent/tests/test_app.py | 174 ++++++++++++++- tests/unit_tests/test_rollout_collection.py | 82 +++++++ tests/unit_tests/test_skills.py | 198 +++++++++++++++++ 9 files changed, 885 insertions(+), 15 deletions(-) create mode 100644 fern/versions/latest/pages/agent-server/agent-skills.mdx create mode 100644 nemo_gym/skills.py create mode 100644 tests/unit_tests/test_skills.py diff --git a/fern/versions/latest/pages/agent-server/agent-skills.mdx b/fern/versions/latest/pages/agent-server/agent-skills.mdx new file mode 100644 index 0000000000..62d7244547 --- /dev/null +++ b/fern/versions/latest/pages/agent-server/agent-skills.mdx @@ -0,0 +1,105 @@ +--- +title: "Agent Skills" +description: "Evaluate agent skills as a run-level variable, decoupled from the dataset" +position: 2 +--- + +# Agent Skills + +Skills are reusable units of operational knowledge an agent can load at runtime, following the open [Agent Skills standard](https://agentskills.io/specification) used by Claude Code and Codex CLI. A skill is a **directory** containing a `SKILL.md` file (YAML frontmatter + markdown body) plus optional supporting files. + +NeMo Gym treats skills as an **environment-level variable you can sweep**, the same way it treats prompts. You point a run at a directory of skills, the agent loads them with its own native mechanism, and each rollout result is tagged so you can compare variants. NeMo Gym does not interpret or activate skills — it sets up the skill environment and measures the outcome. + +## Skills are a run-level knob, not a dataset field + +Skills are specified on `ng_collect_rollouts` and applied to a **fixed, skill-agnostic dataset** at rollout time. The dataset stays a description of tasks, so the same dataset is reusable across skill variants and across agents. + +## Usage + +A single `skills.path` field points at a directory of skill directories: + +```bash +ng_collect_rollouts +agent_name=my_agent \ + +input_jsonl_fpath=data/tasks.jsonl \ + +output_jsonl_fpath=results/rollouts_variant_a.jsonl \ + +skills.path=skills/variant_a/ +``` + +The directory follows the standard layout — one subdirectory per skill, each with a `SKILL.md`: + +``` +skills/variant_a/ +├── cot_enhanced/ +│ └── SKILL.md +├── tool_focused/ +│ ├── SKILL.md +│ └── references/ +│ └── api_spec.md +└── baseline/ + └── SKILL.md +``` + +To compare variants, run again with a different `skills.path` over the same dataset. The skills path is resolved like `input_jsonl_fpath` (relative paths check the working directory, then the Gym root). For distributed runs the directory must be on storage accessible to the agent process. + +## Output: `skills_ref` + +Each rollout result is stamped with a `skills_ref` for provenance and grouping during reward profiling: + +```json +{ + "reward": 1.0, + "skills_ref": { + "path": "skills/variant_a/", + "hash": "a1b2c3…", + "skills": [{"name": "cot_enhanced", "description": "..."}] + } +} +``` + +`hash` is a short content digest of the skill directory. It exists so that optimizer loops (e.g. ACE, GEPA, EvoSkill) that mutate a skill **in place** at the same path still produce distinguishable variants — identity is derived from bytes on disk, requiring no cooperation from the optimizer. Identical content yields an identical hash, so re-testing a prior variant automatically pools with its earlier evaluation. + + +For concurrent candidate evaluation (population search), write each candidate to its own directory (`skills/cand-0/`, `skills/cand-1/`, …) to avoid a path-reuse read/write race. + + +## How skills reach the agent + +NeMo Gym handles everything agent-agnostic: it resolves `skills.path`, computes the `skills_ref`, stamps it onto each request, and propagates it to results. The agent's job is only to make the resolved directory discoverable by its native runtime — because *where* a runtime discovers skills is intrinsically agent-specific. + +This splits cleanly into **shared core** and a **thin per-agent adapter**: + +| Responsibility | Owner | +|---|---| +| `skills.path` config, load/validate, content hash, `skills_ref` stamping + propagation | NeMo Gym core (shared) | +| Read `skills_ref.path` from the request | Per-agent (identical shape) | +| Stage the directory into the runtime's discovery location | Per-agent (location differs) | +| Enable native discovery; load/activate skills | The agent's native runtime | + +## Adding skills support to an agent + +To support skills in a new agent, implement the three-step adapter. NeMo Gym provides the shared utilities in `nemo_gym.skills`. + +1. **Read the skills path from the request.** NeMo Gym stamps `skills_ref` (with `path`) onto the `/run` request body. Read `skills_ref["path"]` in your agent's `run()`. + +2. **Stage the directory where your runtime discovers skills**, using `nemo_gym.skills.stage_skills(path, dest_dir)`. The destination is agent-specific: + + | Agent | Discovery location | + |---|---| + | Claude Code | `CLAUDE_CONFIG_DIR/skills/` | + | Codex CLI | the Codex config home (e.g. `$CODEX_HOME`) | + | MCP-backed agent | configure the MCP skill server to serve from the directory | + + Prefer staging into a **per-request** ephemeral location so concurrent requests with different skills do not contaminate one another, and so nothing leaks between rollouts. + +3. **Enable native discovery.** If your runtime disables discovery by default (Claude Code runs `--bare`), turn it on when skills are present. Then the agent's native loader handles selection and activation — NeMo Gym adds no skill-interpretation logic. + + Note that enabling discovery is usually all-or-nothing: with Claude Code, dropping `--bare` so skills load also re-enables *all* other auto-discovery — hooks, plugins, MCP servers, memory, and `CLAUDE.md` — not just skills. If you are measuring a skill's isolated impact, be aware that a baseline (`--bare`, no skills) versus a skills run may differ by more than the skill content alone. + + +The reference implementation is the [Claude Code agent](https://github.com/NVIDIA-NeMo/Gym/tree/main/responses_api_agents/claude_code_agent). It stages skills into a fresh per-request `CLAUDE_CONFIG_DIR/skills/` and drops `--bare` when skills are active. + + +## What NeMo Gym does not do + +- It does not parse, select, inject, or activate skills — the agent's native runtime does all of that. +- It does not add skill-loading capabilities to agents that lack them. Injecting skill content into the prompt for such agents is a possible future enhancement, not part of this design. diff --git a/nemo_gym/global_config.py b/nemo_gym/global_config.py index c9d7e25aeb..2942223164 100644 --- a/nemo_gym/global_config.py +++ b/nemo_gym/global_config.py @@ -117,6 +117,7 @@ RESPONSES_CREATE_PARAMS_KEY_NAME = "responses_create_params" RESPONSE_KEY_NAME = "response" AGENT_REF_KEY_NAME = "agent_ref" +SKILLS_REF_KEY_NAME = "skills_ref" POLICY_BASE_URL_KEY_NAME = "policy_base_url" POLICY_API_KEY_KEY_NAME = "policy_api_key" # pragma: allowlist secret diff --git a/nemo_gym/rollout_collection.py b/nemo_gym/rollout_collection.py index 8c6c2548f7..617e50a54d 100644 --- a/nemo_gym/rollout_collection.py +++ b/nemo_gym/rollout_collection.py @@ -38,6 +38,7 @@ AGENT_REF_KEY_NAME, RESPONSES_CREATE_PARAMS_KEY_NAME, ROLLOUT_INDEX_KEY_NAME, + SKILLS_REF_KEY_NAME, TASK_INDEX_KEY_NAME, get_wandb_run, ) @@ -51,6 +52,7 @@ raise_for_status, set_global_aiohttp_client, ) +from nemo_gym.skills import SkillsConfig, load_skill_directory # --------------------------------------------------------------------------- @@ -198,6 +200,10 @@ class RolloutCollectionConfig(SharedRolloutCollectionConfig): default=None, description="Path to a prompt YAML file. Builds responses_create_params.input from the template at rollout time. Mutually exclusive with pre-populated responses_create_params.input in the JSONL data.", ) + skills: Optional[SkillsConfig] = Field( + default=None, + description="Run-level skills config (skills.path). Makes a directory of Agent Skills standard skills available to the agent at rollout time and stamps each result with a skills_ref. Applied to a skill-agnostic dataset; not a dataset-row field.", + ) @model_validator(mode="after") def _validate_num_repeats(self) -> "RolloutCollectionConfig": @@ -268,6 +274,17 @@ def _preprocess_rows_from_config(self, config: RolloutCollectionConfig) -> List[ prompt_cfg = load_prompt_config(config.prompt_config) print(f"Using prompt config: {config.prompt_config}") + # Resolve skills once for the whole run (hash is content-derived, computed at startup). + skills_ref_dict = None + if config.skills: + skills_ref = load_skill_directory(config.skills.path) + skills_ref_dict = skills_ref.model_dump() + print( + f"Using skills from {config.skills.path} " + f"(hash={skills_ref.hash}, {len(skills_ref.skills)} skill(s): " + f"{', '.join(s.name for s in skills_ref.skills)})" + ) + _input_path = Path(config.input_jsonl_fpath) if not _input_path.is_absolute(): _cwd_path = Path.cwd() / _input_path @@ -305,6 +322,11 @@ def _preprocess_rows_from_config(self, config: RolloutCollectionConfig) -> List[ row[RESPONSES_CREATE_PARAMS_KEY_NAME] | responses_create_params_overrides ) + # Stamp the run-level skills_ref onto the row so it is sent to the agent in the + # /run request body and propagated to results. The source dataset stays untouched. + if skills_ref_dict is not None: + row[SKILLS_REF_KEY_NAME] = skills_ref_dict + # Resolve task index. Honor a caller-provided value when present (e.g. when an # upstream slicer has stamped a globally-stable index across chunks so that # subsequent /aggregate_metrics groupby unions chunks correctly); otherwise dedupe @@ -467,6 +489,8 @@ async def run_from_config(self, config: RolloutCollectionConfig) -> Tuple[List[D result[TASK_INDEX_KEY_NAME] = row[TASK_INDEX_KEY_NAME] result[ROLLOUT_INDEX_KEY_NAME] = row[ROLLOUT_INDEX_KEY_NAME] result[AGENT_REF_KEY_NAME] = row[AGENT_REF_KEY_NAME] + if SKILLS_REF_KEY_NAME in row: + result[SKILLS_REF_KEY_NAME] = row[SKILLS_REF_KEY_NAME] no_persist = bool(result.get(NG_NO_PERSIST_KEY)) failure_class = result.get(NG_FAILURE_CLASS_KEY) diff --git a/nemo_gym/skills.py b/nemo_gym/skills.py new file mode 100644 index 0000000000..e271b345b7 --- /dev/null +++ b/nemo_gym/skills.py @@ -0,0 +1,201 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# 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. +"""Agent skills: a directory of skills made available to an agent at rollout time. + +Skills follow the open `Agent Skills standard `_ +used by Codex CLI and Claude Code. A skill is a *directory* containing a ``SKILL.md`` +file (YAML frontmatter + markdown body) plus optional supporting files. The ``skills.path`` +config points at a directory of such skill directories. + +Skills are a run-level knob (specified on ``ng_collect_rollouts``), applied to a fixed, +skill-agnostic dataset -- mirroring how ``prompt.py`` applies a prompt template. They are +*not* a dataset-row field, so the same dataset is reusable across skill variants. Each +rollout result is stamped with a ``skills_ref`` for provenance/grouping in reward profiling. + +The ``skills_ref`` carries a content ``hash`` (a short sha256 over the skill directory's +sorted relative paths + file bytes) so that variants that mutate a skill *in place* at the +same path -- as optimizer loops like ACE, GEPA, and EvoSkill commonly do -- remain +distinguishable when comparing rollouts. Identity is derived from bytes on disk, so it +requires no cooperation from the optimizer. +""" + +import hashlib +import shutil +from pathlib import Path +from typing import List, Optional + +import yaml +from pydantic import BaseModel, Field + +from nemo_gym import PARENT_DIR + + +SKILL_MD_FILENAME = "SKILL.md" +# 12 hex chars (48 bits) is plenty to separate the handful of variants in one experiment +# while staying readable in tables / W&B. +_HASH_PREFIX_LEN = 12 + + +class SkillMetadata(BaseModel): + """Metadata parsed from a single skill's ``SKILL.md`` YAML frontmatter.""" + + name: str + description: Optional[str] = None + version: Optional[str] = None + + +class SkillsRef(BaseModel): + """Provenance stamp describing the skills made available for a run. + + Stamped onto rollout result rows (not source datasets). ``hash`` is a content + digest so two skill *versions* at the same ``path`` do not collide in profiling. + """ + + path: str + hash: str + skills: List[SkillMetadata] + + +class SkillsConfig(BaseModel): + """Run-level skills config: ``skills.path`` points at a directory of skill directories.""" + + path: str = Field(description="Directory of Agent Skills standard skill directories to make available.") + + +def _resolve_skills_path(path: str) -> Path: + """Resolve a skills path. Relative paths check cwd first, then the Gym root (PARENT_DIR). + + This matches how ``input_jsonl_fpath`` and ``config_paths`` are resolved. + """ + p = Path(path) + if p.is_absolute(): + return p + cwd_path = Path.cwd() / p + return cwd_path if cwd_path.exists() else PARENT_DIR / p + + +def hash_skill_dir(root: Path) -> str: + """Compute a stable short sha256 over a skill directory's contents. + + Walks files in sorted relative-path order, folding each file's relative path and bytes + into the digest. Including the relative path means renaming or adding/removing files also + changes the hash -- a skill *is* its file layout, not just ``SKILL.md``'s bytes. + """ + h = hashlib.sha256() + for file_path in sorted(p for p in root.rglob("*") if p.is_file()): + h.update(file_path.relative_to(root).as_posix().encode()) + h.update(b"\0") + h.update(file_path.read_bytes()) + h.update(b"\0") + return h.hexdigest()[:_HASH_PREFIX_LEN] + + +def parse_skill_md(skill_md_path: Path) -> SkillMetadata: + """Parse a ``SKILL.md`` file's YAML frontmatter into ``SkillMetadata``. + + Frontmatter is delimited by lines containing only ``---``. Raises ``ValueError`` with a + clear message if the frontmatter is missing, malformed, or lacks a ``name``. + """ + # utf-8-sig transparently strips a leading BOM if present; Claude Code's own loader tolerates + # one, so a BOM should not make Gym reject a skill that would otherwise run. + text = skill_md_path.read_text(encoding="utf-8-sig", errors="replace") + lines = text.splitlines() + if not lines or lines[0].strip() != "---": + raise ValueError( + f"Skill file {skill_md_path} is missing YAML frontmatter. " + f"It must start with a '---' line followed by 'name:' and 'description:' fields." + ) + + closing_idx = None + for idx in range(1, len(lines)): + if lines[idx].strip() == "---": + closing_idx = idx + break + if closing_idx is None: + raise ValueError( + f"Skill file {skill_md_path} has an unterminated YAML frontmatter block (no closing '---' line)." + ) + + frontmatter_text = "\n".join(lines[1:closing_idx]) + try: + data = yaml.safe_load(frontmatter_text) or {} + except yaml.YAMLError as e: + raise ValueError(f"Skill file {skill_md_path} has malformed YAML frontmatter: {e}") from None + if not isinstance(data, dict): + raise ValueError(f"Skill file {skill_md_path} frontmatter must be a YAML mapping, got {type(data).__name__}.") + if not data.get("name"): + raise ValueError(f"Skill file {skill_md_path} frontmatter is missing a required 'name' field.") + + # The Agent Skills spec nests version under an optional `metadata:` map + # (e.g. `metadata: {version: "1.0"}`), not as a top-level key. + metadata = data.get("metadata") + version = str(metadata["version"]) if isinstance(metadata, dict) and metadata.get("version") is not None else None + return SkillMetadata( + name=str(data["name"]), + description=data.get("description"), + version=version, + ) + + +def load_skill_directory(path: str) -> SkillsRef: + """Load a directory of skills, returning a ``SkillsRef`` (path, content hash, metadata). + + The directory at ``path`` contains one subdirectory per skill, each with a ``SKILL.md``. + Raises ``ValueError`` with an actionable message if the path is missing, is not a + directory, contains no skills, or contains a malformed skill. + """ + resolved = _resolve_skills_path(path) + if not resolved.exists(): + raise ValueError(f"Skills path does not exist: {resolved} (from skills.path={path!r}).") + if not resolved.is_dir(): + raise ValueError( + f"Skills path must be a directory of skill directories, but {resolved} is a file " + f"(from skills.path={path!r})." + ) + + skill_dirs = sorted(d for d in resolved.iterdir() if d.is_dir()) + skills: List[SkillMetadata] = [] + for skill_dir in skill_dirs: + skill_md = skill_dir / SKILL_MD_FILENAME + if not skill_md.is_file(): + raise ValueError( + f"Skill directory {skill_dir} is missing a {SKILL_MD_FILENAME} file. " + f"Each skill must be a directory containing a {SKILL_MD_FILENAME}." + ) + skills.append(parse_skill_md(skill_md)) + + if not skills: + raise ValueError( + f"Skills path {resolved} contains no skills. Expected one or more subdirectories, " + f"each containing a {SKILL_MD_FILENAME} file (from skills.path={path!r})." + ) + + return SkillsRef(path=path, hash=hash_skill_dir(resolved), skills=skills) + + +def stage_skills(path: str, dest_skills_dir: Path) -> None: + """Copy the directory of skills at ``path`` into ``dest_skills_dir``. + + Used by agent runtimes to materialize skills into a location their native discovery + mechanism scans (e.g. ``/skills/`` for Claude Code). ``dest_skills_dir`` + must not already exist. Raises ``ValueError`` if the source path is missing or not a directory. + """ + resolved = _resolve_skills_path(path) + if not resolved.is_dir(): + raise ValueError( + f"Cannot stage skills: {resolved} is not a directory (from skills path {path!r}). " + f"For distributed runs the skills directory must be on storage accessible to the agent." + ) + shutil.copytree(resolved, dest_skills_dir) diff --git a/responses_api_agents/claude_code_agent/README.md b/responses_api_agents/claude_code_agent/README.md index 94a0fabf3f..3eb0456573 100644 --- a/responses_api_agents/claude_code_agent/README.md +++ b/responses_api_agents/claude_code_agent/README.md @@ -142,6 +142,55 @@ The agent defaults to a plain `bare` CLI call for simplicity and reproducibility The per-run `CLAUDE_CONFIG_DIR` is created fresh for each request and removed afterward, so opted-in content is staged per rollout and does not leak between runs. This is the staging seam reused by skills evaluation (placing skills under `CLAUDE_CONFIG_DIR/skills/`). +## Skills evaluation + +Skills are evaluated as a run-level variable, not a dataset field — the same skill-agnostic dataset is reused across skill variants (mirroring how `prompt_config` works). You point `skills.path` at a directory of [Agent Skills standard](https://agentskills.io/specification) skill directories on `ng_collect_rollouts`, and the agent stages them into each request's `CLAUDE_CONFIG_DIR/skills/` so Claude Code's native discovery picks them up. When skills are present, `--bare` is forced off for that request regardless of the `bare` config. + +Expected layout (each skill is a directory with a `SKILL.md`): + +``` +skills/variant_a/ +├── cot_enhanced/ +│ └── SKILL.md +├── tool_focused/ +│ ├── SKILL.md +│ └── references/ +│ └── api_spec.md +└── baseline/ + └── SKILL.md +``` + +Compare two variants over the same dataset by changing only `skills.path`: + +```bash +ng_collect_rollouts +agent_name=reasoning_gym_claude_code_agent \ + +input_jsonl_fpath=resources_servers/reasoning_gym/data/example.jsonl \ + +output_jsonl_fpath=rollouts_variant_a.jsonl \ + +skills.path=skills/variant_a/ + +ng_collect_rollouts +agent_name=reasoning_gym_claude_code_agent \ + +input_jsonl_fpath=resources_servers/reasoning_gym/data/example.jsonl \ + +output_jsonl_fpath=rollouts_variant_b.jsonl \ + +skills.path=skills/variant_b/ +``` + +Each rollout result is stamped with a `skills_ref` for provenance and grouping during reward profiling: + +```json +{ + "reward": 1.0, + "skills_ref": { + "path": "skills/variant_a/", + "hash": "a1b2c3…", + "skills": [{"name": "cot_enhanced", "description": "..."}] + } +} +``` + +`hash` is a content digest of the skill directory, so optimizer loops (e.g. ACE, GEPA, EvoSkill) that mutate a skill **in place** at the same path still produce distinguishable variants. For concurrent candidate evaluation, give each candidate its own directory (`skills/cand-0/`, `skills/cand-1/`, …) to avoid a path-reuse read/write race. + +The skills path is resolved like `input_jsonl_fpath` (relative paths check the working directory, then the Gym root). For distributed runs the directory must be on storage accessible to the agent process. + ## Limitations - Eval only for now. Token IDs and logprobs are not wired up yet. diff --git a/responses_api_agents/claude_code_agent/app.py b/responses_api_agents/claude_code_agent/app.py index 93be891a33..6970d92ce1 100644 --- a/responses_api_agents/claude_code_agent/app.py +++ b/responses_api_agents/claude_code_agent/app.py @@ -37,7 +37,7 @@ SimpleResponsesAPIAgent, ) from nemo_gym.config_types import ModelServerRef, ResourcesServerRef -from nemo_gym.global_config import get_first_server_config_dict +from nemo_gym.global_config import SKILLS_REF_KEY_NAME, get_first_server_config_dict from nemo_gym.openai_utils import ( NeMoGymEasyInputMessage, NeMoGymFunctionCallOutput, @@ -51,6 +51,7 @@ NeMoGymResponseUsage, ) from nemo_gym.server_utils import get_response_json, raise_for_status +from nemo_gym.skills import stage_skills from responses_api_agents.claude_code_agent.setup_claude_code import ensure_claude_code @@ -292,16 +293,26 @@ def _build_settings(self) -> dict[str, Any]: settings = {**settings, **user_settings, "env": {**settings["env"], **user_env}} return settings - def _setup_config_dir(self) -> Path: - """Create a per-run CLAUDE_CONFIG_DIR and stage settings into it. + def _setup_config_dir(self, skills_path: Optional[str] = None) -> Path: + """Create a per-run CLAUDE_CONFIG_DIR and stage settings (and optionally skills) into it. - The directory lives for the duration of a single ``_run_claude_code`` call and is the - staging seam for capabilities discovered from CLAUDE_CONFIG_DIR (e.g. skills under - ``/skills/``). The caller is responsible for removing it. + The directory lives for the duration of a single ``_run_claude_code`` call. When + ``skills_path`` is provided, the directory of skills is copied into ``/skills/`` so + Claude Code's native discovery can pick them up. Each request gets its own ephemeral copy, + so concurrent requests with different skills do not contaminate one another. The caller is + responsible for removing the directory on success; if setup fails partway (e.g. a bad + ``skills_path``), this method cleans up the partially-created dir before re-raising so it + does not leak (the caller never receives the path in that case). """ claude_config_dir = Path.home() / ".claude_code_agent" / uuid4().hex claude_config_dir.mkdir(parents=True) - (claude_config_dir / "settings.json").write_text(json.dumps(self._build_settings())) + try: + (claude_config_dir / "settings.json").write_text(json.dumps(self._build_settings())) + if skills_path: + stage_skills(skills_path, claude_config_dir / "skills") + except Exception: + shutil.rmtree(claude_config_dir, ignore_errors=True) + raise return claude_config_dir def _build_command( @@ -310,6 +321,7 @@ def _build_command( instruction: str, system_prompt: Optional[str] = None, mcp_config: Optional[str] = None, + skills_active: bool = False, ) -> list[str]: """Construct the ``claude`` CLI argv from config. @@ -317,6 +329,9 @@ def _build_command( attribution, auto-memory, background prefetches, keychain reads, and CLAUDE.md auto-discovery (skills still resolve via /skill-name). Explicit capabilities like ``--mcp-config`` are passed regardless of ``--bare`` since they are not auto-discovered. + + When ``skills_active`` is True (skills were staged into CLAUDE_CONFIG_DIR for this request), + ``--bare`` is forced off so Claude Code's native filesystem discovery picks the skills up. """ cmd = [ "claude", @@ -326,7 +341,13 @@ def _build_command( "--verbose", "--dangerously-skip-permissions", ] - if self.config.bare: + if self.config.bare and skills_active: + LOG.warning( + "skills are active for this request; ignoring bare=True so Claude Code can discover them. " + "Note this re-enables ALL native auto-discovery, not just skills (hooks, plugins, MCP servers, " + "memory, and CLAUDE.md), so the runtime broadens versus a bare baseline." + ) + if self.config.bare and not skills_active: cmd.append("--bare") cmd += ["--max-turns", str(self.config.max_turns), "--model", model] effective_mcp_config = mcp_config if mcp_config is not None else self.config.mcp_config @@ -350,6 +371,7 @@ async def _run_claude_code( instruction: str, system_prompt: Optional[str] = None, mcp_config: Optional[str] = None, + skills_path: Optional[str] = None, ) -> tuple[str, str]: """Run claude -p --output-format=stream-json and return (stdout, model_name).""" base_url = self._resolve_base_url() @@ -357,8 +379,11 @@ async def _run_claude_code( model = self.config.model if base_url else self.config.model.split("/")[-1] api_key = self.config.anthropic_api_key - claude_config_dir = self._setup_config_dir() + claude_config_dir = None try: + # Inside the try so a bad skills.path (raising in stage_skills) still cleans up the + # partially-created config dir in the finally rather than leaking it per failing request. + claude_config_dir = self._setup_config_dir(skills_path=skills_path) env = { **os.environ, "ANTHROPIC_API_KEY": api_key, # pragma: allowlist secret @@ -374,7 +399,13 @@ async def _run_claude_code( env["ANTHROPIC_BASE_URL"] = base_url env["ANTHROPIC_AUTH_TOKEN"] = api_key or "local" - cmd = self._build_command(model, instruction, system_prompt=system_prompt, mcp_config=mcp_config) + cmd = self._build_command( + model, + instruction, + system_prompt=system_prompt, + mcp_config=mcp_config, + skills_active=bool(skills_path), + ) proc = await asyncio.create_subprocess_exec( *cmd, @@ -396,7 +427,8 @@ async def _run_claude_code( LOG.debug("claude-code stdout (%d chars): %s", len(stdout), stdout[:2000].decode(errors="replace")) return stdout.decode(errors="replace"), model finally: - shutil.rmtree(claude_config_dir, ignore_errors=True) + if claude_config_dir is not None: + shutil.rmtree(claude_config_dir, ignore_errors=True) def _resources_server_base_url(self) -> str: cfg = get_first_server_config_dict( @@ -462,6 +494,7 @@ async def _create_response( self, body: NeMoGymResponseCreateParamsNonStreaming, mcp_config: Optional[str] = None, + skills_path: Optional[str] = None, ) -> NeMoGymResponse: body = body.model_copy(deep=True) if isinstance(body.input, str): @@ -475,6 +508,7 @@ async def _create_response( user_message, system_prompt=system_prompt, mcp_config=mcp_config, + skills_path=skills_path, ) output_items, usage = parse_stream_json(stdout) @@ -535,9 +569,17 @@ async def run(self, request: Request, body: ClaudeCodeAgentRunRequest) -> Claude cookies = seed_resp.cookies seed_resp_json = await get_response_json(seed_resp) + # The run-level skills_ref (stamped by rollout collection) rides on the request body + # (extra="allow"). Pass its path straight into _create_response so the CLI invocation + # can stage the skills into its per-request CLAUDE_CONFIG_DIR. run() calls _create_response + # in-process, so no metadata side-channel is needed (unlike the schema-forbidden HTTP path). + skills_path = ((body.model_extra or {}).get(SKILLS_REF_KEY_NAME) or {}).get("path") + with tempfile.TemporaryDirectory(prefix="nemo_gym_claude_mcp_") as mcp_config_dir: mcp_config = self._write_rollout_mcp_config(seed_resp_json, Path(mcp_config_dir)) - agent_resp = await self._create_response(body.responses_create_params, mcp_config=mcp_config) + agent_resp = await self._create_response( + body.responses_create_params, mcp_config=mcp_config, skills_path=skills_path + ) agent_resp_json = agent_resp.model_dump(mode="json") verify_resp = await self.server_client.post( diff --git a/responses_api_agents/claude_code_agent/tests/test_app.py b/responses_api_agents/claude_code_agent/tests/test_app.py index ae024d5fdc..4c12003b0e 100644 --- a/responses_api_agents/claude_code_agent/tests/test_app.py +++ b/responses_api_agents/claude_code_agent/tests/test_app.py @@ -16,11 +16,14 @@ import asyncio import json from pathlib import Path -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch +import orjson +import pytest import yaml from fastapi import Request +from nemo_gym.global_config import SKILLS_REF_KEY_NAME from nemo_gym.openai_utils import ( NeMoGymEasyInputMessage, NeMoGymFunctionCallOutput, @@ -39,6 +42,14 @@ ) +def _write_skill_dir(root: Path, name: str = "cot_enhanced") -> Path: + skills_dir = root / "variant_a" + skill = skills_dir / name + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text(f"---\nname: {name}\ndescription: A skill.\n---\n# Body\n") + return skills_dir + + def _config(**kwargs) -> ClaudeCodeAgentConfig: kwargs.setdefault("resources_server", ResourcesServerRef(type="resources_servers", name="")) return ClaudeCodeAgentConfig( @@ -125,6 +136,17 @@ def test_dynamic_mcp_config_overrides_static_for_command(self) -> None: cmd = agent._build_command("m", "x", mcp_config="/tmp/dynamic.json") assert cmd[cmd.index("--mcp-config") + 1] == "/tmp/dynamic.json" + def test_skills_active_forces_bare_off(self) -> None: + # Default config has bare=True, but staged skills must be discoverable, so --bare is dropped. + agent = _make_agent() + cmd = agent._build_command("m", "x", skills_active=True) + assert "--bare" not in cmd + + def test_skills_inactive_keeps_bare(self) -> None: + agent = _make_agent() + cmd = agent._build_command("m", "x", skills_active=False) + assert "--bare" in cmd + def test_optional_flags_threaded_through(self) -> None: agent = _make_agent( allowed_tools="Bash,Read", @@ -184,6 +206,109 @@ def test_creates_dir_with_settings(self, tmp_path: Path) -> None: _shutil.rmtree(config_dir, ignore_errors=True) + def test_stages_skills_into_config_dir(self, tmp_path: Path) -> None: + skills_dir = _write_skill_dir(tmp_path) + home = tmp_path / "home" + home.mkdir() + agent = _make_agent() + with patch("responses_api_agents.claude_code_agent.app.Path.home", return_value=home): + config_dir = agent._setup_config_dir(skills_path=str(skills_dir)) + try: + assert (config_dir / "skills" / "cot_enhanced" / "SKILL.md").is_file() + finally: + import shutil as _shutil + + _shutil.rmtree(config_dir, ignore_errors=True) + + +class _FakeHttpResp: + def __init__(self, payload: dict) -> None: + self._payload = payload + self.cookies: dict = {} + self.ok = True + + async def read(self) -> bytes: + return orjson.dumps(self._payload) + + +def _gym_response(text: str = "done") -> dict: + return { + "id": "resp_x", + "created_at": 0.0, + "model": "claude-sonnet-4-6", + "object": "response", + "output": [ + { + "id": "msg_x", + "content": [{"annotations": [], "text": text, "type": "output_text"}], + "role": "assistant", + "status": "completed", + "type": "message", + } + ], + "parallel_tool_calls": True, + "tool_choice": "auto", + "tools": [], + "usage": { + "input_tokens": 1, + "input_tokens_details": {"cached_tokens": 0}, + "output_tokens": 1, + "output_tokens_details": {"reasoning_tokens": 0}, + "total_tokens": 2, + }, + } + + +class TestRunForwardsSkillsPath: + """run() reads skills_ref off the request's model_extra (extra='allow') and forwards its path + directly to _create_response/_run_claude_code.""" + + def _seed_and_verify_post(self): + async def _post(server_name, url_path, json=None, cookies=None, **kw): + if url_path == "/verify": + return _FakeHttpResp( + {"responses_create_params": {"input": []}, "response": _gym_response(), "reward": 1.0} + ) + return _FakeHttpResp({}) + + return AsyncMock(side_effect=_post) + + def _run(self, agent: ClaudeCodeAgent, body: ClaudeCodeAgentRunRequest, run_claude_code: AsyncMock): + agent.server_client.post = self._seed_and_verify_post() + req = MagicMock() + req.cookies = {} + # Stub the CLI invocation; _create_response still runs for real, so we exercise the full + # run() -> _create_response -> _run_claude_code argument threading. + with patch.object( + ClaudeCodeAgent, + "_run_claude_code", + run_claude_code, + ): + return asyncio.run(agent.run(req, body)) + + def test_skills_ref_path_forwarded(self) -> None: + agent = _make_agent() + run_claude_code = AsyncMock(return_value=("", "claude-sonnet-4-6")) + body = ClaudeCodeAgentRunRequest.model_validate( + { + "responses_create_params": {"input": []}, + SKILLS_REF_KEY_NAME: {"path": "skills/variant_a/", "hash": "abc123", "skills": []}, + } + ) + + self._run(agent, body, run_claude_code) + + assert run_claude_code.call_args.kwargs["skills_path"] == "skills/variant_a/" + + def test_no_skills_ref_forwards_none(self) -> None: + agent = _make_agent() + run_claude_code = AsyncMock(return_value=("", "claude-sonnet-4-6")) + body = ClaudeCodeAgentRunRequest.model_validate({"responses_create_params": {"input": []}}) + + self._run(agent, body, run_claude_code) + + assert run_claude_code.call_args.kwargs["skills_path"] is None + class TestRunClaudeCode: def test_wires_command_env_and_cleans_up(self, tmp_path: Path) -> None: @@ -222,6 +347,49 @@ async def fake_exec(*cmd, **kwargs): assert "result" in stdout assert model == "claude-sonnet-4-6" + def test_skills_staged_and_bare_dropped(self, tmp_path: Path) -> None: + skills_dir = _write_skill_dir(tmp_path) + home = tmp_path / "home" + home.mkdir() + agent = _make_agent() # bare defaults to True + captured: dict = {} + + class FakeProc: + returncode = 0 + + async def communicate(self): + return b'{"type":"result","usage":{"input_tokens":1,"output_tokens":1}}\n', b"" + + async def fake_exec(*cmd, **kwargs): + config_dir = Path(kwargs["env"]["CLAUDE_CONFIG_DIR"]) + captured["cmd"] = list(cmd) + captured["skill_staged"] = (config_dir / "skills" / "cot_enhanced" / "SKILL.md").is_file() + return FakeProc() + + with ( + patch("responses_api_agents.claude_code_agent.app.Path.home", return_value=home), + patch("responses_api_agents.claude_code_agent.app.asyncio.create_subprocess_exec", fake_exec), + ): + asyncio.run(agent._run_claude_code("hello", skills_path=str(skills_dir))) + + assert captured["skill_staged"] is True + # skills present => --bare must be dropped even though config.bare is True + assert "--bare" not in captured["cmd"] + + def test_bad_skills_path_does_not_leak_config_dir(self, tmp_path: Path) -> None: + # stage_skills raises for a missing skills dir; the partially-created config dir must + # still be cleaned up (setup happens inside the try whose finally rmtree's it). + home = tmp_path / "home" + home.mkdir() + agent = _make_agent() + + with patch("responses_api_agents.claude_code_agent.app.Path.home", return_value=home): + with pytest.raises(ValueError): + asyncio.run(agent._run_claude_code("hello", skills_path=str(tmp_path / "does_not_exist"))) + + leaked = home / ".claude_code_agent" + assert not leaked.exists() or not any(leaked.iterdir()) + def test_timeout_returns_empty(self, tmp_path: Path) -> None: agent = _make_agent(timeout=1) killed = {"called": False} @@ -357,7 +525,7 @@ async def fake_post(server_name, url_path, json=None, cookies=None): captured: dict = {} - async def fake_run_claude_code(instruction, system_prompt=None, mcp_config=None): + async def fake_run_claude_code(instruction, system_prompt=None, mcp_config=None, skills_path=None): captured["instruction"] = instruction captured["mcp_config"] = mcp_config captured["config_exists_during_run"] = Path(mcp_config).is_file() @@ -413,7 +581,7 @@ async def fake_post(server_name, url_path, json=None, cookies=None): return FakeAioHTTPResponse(json | {"reward": 1.0}) raise AssertionError(f"unexpected post: {server_name} {url_path}") - async def fake_run_claude_code(instruction, system_prompt=None, mcp_config=None): + async def fake_run_claude_code(instruction, system_prompt=None, mcp_config=None, skills_path=None): captured["config_token"] = json.loads(Path(mcp_config).read_text())["mcpServers"]["example_mcp_weather"][ "headers" ]["X-NeMo-Gym-Session-Token"] diff --git a/tests/unit_tests/test_rollout_collection.py b/tests/unit_tests/test_rollout_collection.py index da19728e92..c6db6da6a1 100644 --- a/tests/unit_tests/test_rollout_collection.py +++ b/tests/unit_tests/test_rollout_collection.py @@ -283,6 +283,88 @@ def test_preprocess_rows_from_config(self, tmp_path: Path) -> None: }, ] + def test_preprocess_rows_stamps_skills_ref(self, tmp_path: Path) -> None: + """skills.path is a run-level knob: each row is stamped with skills_ref (path + hash + + metadata) without the source dataset carrying any skills field.""" + skills_dir = tmp_path / "variant_a" + skill = skills_dir / "cot_enhanced" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("---\nname: cot_enhanced\ndescription: Think step by step.\n---\n# Body\n") + + fpath = tmp_path / "input.jsonl" + samples = [json.dumps({"responses_create_params": {"input": []}, "x": i}) for i in range(2)] + fpath.write_text("\n".join(samples) + "\n") + + config = RolloutCollectionConfig( + agent_name="my_agent", + input_jsonl_fpath=str(fpath), + output_jsonl_fpath=str(tmp_path / "out.jsonl"), + skills={"path": str(skills_dir)}, + ) + + rows = RolloutCollectionHelper._preprocess_rows_from_config(None, config) + + assert len(rows) == 2 + for row in rows: + skills_ref = row["skills_ref"] + assert skills_ref["path"] == str(skills_dir) + assert len(skills_ref["hash"]) == 12 + assert [s["name"] for s in skills_ref["skills"]] == ["cot_enhanced"] + assert skills_ref["skills"][0]["description"] == "Think step by step." + + def test_preprocess_rows_no_skills_leaves_rows_clean(self, tmp_path: Path) -> None: + fpath = tmp_path / "input.jsonl" + fpath.write_text(json.dumps({"responses_create_params": {"input": []}}) + "\n") + config = RolloutCollectionConfig( + agent_name="my_agent", + input_jsonl_fpath=str(fpath), + output_jsonl_fpath=str(tmp_path / "out.jsonl"), + ) + rows = RolloutCollectionHelper._preprocess_rows_from_config(None, config) + assert "skills_ref" not in rows[0] + + def test_skills_ref_survives_resume_from_cache(self, tmp_path: Path) -> None: + """skills_ref is stamped once at preprocess, persisted to materialized inputs, and + re-read onto already-done rows on resume -- even after the source skill dir is gone. + Identity is byte-for-byte from the materialized cache, not recomputed at resume.""" + import shutil + + skills_dir = tmp_path / "variant_a" + skill = skills_dir / "cot_enhanced" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text("---\nname: cot_enhanced\ndescription: Think step by step.\n---\n# Body\n") + + fpath = tmp_path / "input.jsonl" + samples = [json.dumps({"responses_create_params": {"input": []}, "x": i}) for i in range(2)] + fpath.write_text("\n".join(samples) + "\n") + + config = RolloutCollectionConfig( + agent_name="my_agent", + input_jsonl_fpath=str(fpath), + output_jsonl_fpath=str(tmp_path / "out.jsonl"), + skills={"path": str(skills_dir)}, + resume_from_cache=True, + ) + + # Preprocess stamps skills_ref, then we persist exactly what a prior run would have written. + rows = RolloutCollectionHelper._preprocess_rows_from_config(None, config) + stamped_skills_ref = rows[0]["skills_ref"] + config.materialized_jsonl_fpath.write_bytes(b"\n".join(orjson.dumps(r) for r in rows) + b"\n") + + # Only the first task's rollout is "done" in the main output jsonl. + done = {k: rows[0][k] for k in (TASK_INDEX_KEY_NAME, ROLLOUT_INDEX_KEY_NAME)} | {"reward": 1.0} + Path(config.output_jsonl_fpath).write_bytes(orjson.dumps(done) + b"\n") + + # The source skill dir disappears before resume (e.g. an optimizer overwrote /tmp). + shutil.rmtree(skills_dir) + + input_rows, resumed_rows, _results, _result_strs = RolloutCollectionHelper()._load_from_cache(config) + + # The already-done row carries the original skills_ref read back from the cache. + assert resumed_rows[0]["skills_ref"] == stamped_skills_ref + # And the still-to-run rows do too, so the second pass stamps results identically. + assert all(r["skills_ref"] == stamped_skills_ref for r in input_rows) + def test_preprocess_rows_num_repeats_add_seed_passes_pydantic_validation(self, tmp_path: Path) -> None: """Rows emitted with num_repeats_add_seed=True must round-trip through the strict NeMoGymResponseCreateParamsNonStreaming schema (extra='forbid'). Seed is passed via diff --git a/tests/unit_tests/test_skills.py b/tests/unit_tests/test_skills.py new file mode 100644 index 0000000000..192b060d67 --- /dev/null +++ b/tests/unit_tests/test_skills.py @@ -0,0 +1,198 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# 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. + +import pytest + +from nemo_gym import PARENT_DIR +from nemo_gym.skills import ( + _resolve_skills_path, + hash_skill_dir, + load_skill_directory, + parse_skill_md, + stage_skills, +) + + +def _write_skill(skills_dir, name, description="A skill.", version=None, body="# Body\n"): + skill_dir = skills_dir / name + skill_dir.mkdir(parents=True) + frontmatter = f"name: {name}\ndescription: {description}\n" + if version is not None: + # The Agent Skills spec nests version under an optional `metadata:` map. + frontmatter += f"metadata:\n version: {version}\n" + (skill_dir / "SKILL.md").write_text(f"---\n{frontmatter}---\n{body}") + return skill_dir + + +class TestParseSkillMd: + def test_parses_name_description_version(self, tmp_path): + skill_dir = _write_skill(tmp_path, "cot_enhanced", description="Chain of thought.", version="1.2") + meta = parse_skill_md(skill_dir / "SKILL.md") + assert meta.name == "cot_enhanced" + assert meta.description == "Chain of thought." + assert meta.version == "1.2" + + def test_version_optional(self, tmp_path): + skill_dir = _write_skill(tmp_path, "baseline") + meta = parse_skill_md(skill_dir / "SKILL.md") + assert meta.version is None + + def test_top_level_version_ignored(self, tmp_path): + # The spec puts version under metadata:, so a stray top-level version: is not read. + skill_md = tmp_path / "SKILL.md" + skill_md.write_text("---\nname: foo\ndescription: bar\nversion: 9.9\n---\n# Body\n") + meta = parse_skill_md(skill_md) + assert meta.version is None + + def test_bom_prefixed_frontmatter_parses(self, tmp_path): + # A SKILL.md saved with a UTF-8 BOM must not be misread as missing frontmatter. + skill_md = tmp_path / "SKILL.md" + skill_md.write_bytes(b"\xef\xbb\xbf" + b"---\nname: foo\ndescription: bar\n---\n# Body\n") + meta = parse_skill_md(skill_md) + assert meta.name == "foo" + assert meta.description == "bar" + + def test_missing_frontmatter_raises(self, tmp_path): + skill_md = tmp_path / "SKILL.md" + skill_md.write_text("# Just markdown, no frontmatter\n") + with pytest.raises(ValueError, match="missing YAML frontmatter"): + parse_skill_md(skill_md) + + def test_unterminated_frontmatter_raises(self, tmp_path): + skill_md = tmp_path / "SKILL.md" + skill_md.write_text("---\nname: foo\ndescription: bar\n") + with pytest.raises(ValueError, match="unterminated YAML frontmatter"): + parse_skill_md(skill_md) + + def test_malformed_yaml_raises(self, tmp_path): + skill_md = tmp_path / "SKILL.md" + skill_md.write_text("---\nname: : : bad\n\t- nope\n---\n") + with pytest.raises(ValueError, match="malformed YAML frontmatter"): + parse_skill_md(skill_md) + + def test_non_mapping_frontmatter_raises(self, tmp_path): + skill_md = tmp_path / "SKILL.md" + skill_md.write_text("---\n- a\n- b\n---\n") + with pytest.raises(ValueError, match="must be a YAML mapping"): + parse_skill_md(skill_md) + + def test_missing_name_raises(self, tmp_path): + skill_md = tmp_path / "SKILL.md" + skill_md.write_text("---\ndescription: no name here\n---\n") + with pytest.raises(ValueError, match="missing a required 'name' field"): + parse_skill_md(skill_md) + + +class TestHashSkillDir: + def test_stable_across_calls(self, tmp_path): + _write_skill(tmp_path, "a") + assert hash_skill_dir(tmp_path) == hash_skill_dir(tmp_path) + + def test_changes_with_content(self, tmp_path): + skill_dir = _write_skill(tmp_path, "a", description="original") + h1 = hash_skill_dir(tmp_path) + (skill_dir / "SKILL.md").write_text("---\nname: a\ndescription: mutated\n---\n# Body\n") + h2 = hash_skill_dir(tmp_path) + assert h1 != h2 + + def test_changes_with_file_layout(self, tmp_path): + skill_dir = _write_skill(tmp_path, "a") + h1 = hash_skill_dir(tmp_path) + (skill_dir / "references").mkdir() + (skill_dir / "references" / "extra.md").write_text("more context") + h2 = hash_skill_dir(tmp_path) + assert h1 != h2 + + def test_identical_content_same_hash(self, tmp_path): + dir_a = tmp_path / "a" + dir_b = tmp_path / "b" + _write_skill(dir_a, "skill", description="same") + _write_skill(dir_b, "skill", description="same") + assert hash_skill_dir(dir_a) == hash_skill_dir(dir_b) + + def test_hash_is_short_hex(self, tmp_path): + _write_skill(tmp_path, "a") + h = hash_skill_dir(tmp_path) + assert len(h) == 12 + int(h, 16) # parses as hex + + +class TestLoadSkillDirectory: + def test_loads_multiple_skills_sorted(self, tmp_path): + skills_dir = tmp_path / "variant_a" + _write_skill(skills_dir, "tool_focused", description="Tools.") + _write_skill(skills_dir, "baseline", description="Baseline.") + ref = load_skill_directory(str(skills_dir)) + assert ref.path == str(skills_dir) + assert len(ref.hash) == 12 + assert [s.name for s in ref.skills] == ["baseline", "tool_focused"] + assert ref.skills[0].description == "Baseline." + + def test_missing_path_raises(self, tmp_path): + with pytest.raises(ValueError, match="does not exist"): + load_skill_directory(str(tmp_path / "nope")) + + def test_file_instead_of_dir_raises(self, tmp_path): + f = tmp_path / "skills.txt" + f.write_text("not a dir") + with pytest.raises(ValueError, match="must be a directory"): + load_skill_directory(str(f)) + + def test_empty_dir_raises(self, tmp_path): + empty = tmp_path / "empty" + empty.mkdir() + with pytest.raises(ValueError, match="contains no skills"): + load_skill_directory(str(empty)) + + def test_skill_dir_missing_skill_md_raises(self, tmp_path): + skills_dir = tmp_path / "variant" + (skills_dir / "broken").mkdir(parents=True) + with pytest.raises(ValueError, match="missing a SKILL.md"): + load_skill_directory(str(skills_dir)) + + def test_serializable_ref(self, tmp_path): + skills_dir = tmp_path / "variant" + _write_skill(skills_dir, "a") + ref = load_skill_directory(str(skills_dir)) + dumped = ref.model_dump() + assert dumped["path"] == str(skills_dir) + assert dumped["skills"][0]["name"] == "a" + + +class TestResolveSkillsPath: + def test_absolute_path_unchanged(self, tmp_path): + assert _resolve_skills_path(str(tmp_path)) == tmp_path + + def test_relative_resolves_to_parent_dir_when_absent_in_cwd(self): + resolved = _resolve_skills_path("definitely_not_a_real_skills_dir_xyz") + assert resolved == PARENT_DIR / "definitely_not_a_real_skills_dir_xyz" + + +class TestStageSkills: + def test_copies_tree(self, tmp_path): + src = tmp_path / "variant" + _write_skill(src, "cot", description="CoT.") + (src / "cot" / "references").mkdir() + (src / "cot" / "references" / "ref.md").write_text("ref") + + dest = tmp_path / "config" / "skills" + stage_skills(str(src), dest) + + assert (dest / "cot" / "SKILL.md").is_file() + assert (dest / "cot" / "references" / "ref.md").read_text() == "ref" + + def test_missing_source_raises(self, tmp_path): + with pytest.raises(ValueError, match="is not a directory"): + stage_skills(str(tmp_path / "nope"), tmp_path / "dest") From 55bcd6d753c0631a8597ca2c0eb74f309b5c95a2 Mon Sep 17 00:00:00 2001 From: Chris Wing Date: Thu, 25 Jun 2026 20:53:11 -0700 Subject: [PATCH 2/2] docs: update agent-skills page to use gym eval run command Replace the deprecated ng_collect_rollouts invocation with the current gym eval run syntax and note --no-serve usage against already-started servers. Signed-off-by: Chris Wing --- fern/versions/latest/pages/agent-server/agent-skills.mdx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fern/versions/latest/pages/agent-server/agent-skills.mdx b/fern/versions/latest/pages/agent-server/agent-skills.mdx index 62d7244547..8e2afc5985 100644 --- a/fern/versions/latest/pages/agent-server/agent-skills.mdx +++ b/fern/versions/latest/pages/agent-server/agent-skills.mdx @@ -12,14 +12,14 @@ NeMo Gym treats skills as an **environment-level variable you can sweep**, the s ## Skills are a run-level knob, not a dataset field -Skills are specified on `ng_collect_rollouts` and applied to a **fixed, skill-agnostic dataset** at rollout time. The dataset stays a description of tasks, so the same dataset is reusable across skill variants and across agents. +Skills are specified on `gym eval run` and applied to a **fixed, skill-agnostic dataset** at rollout time. The dataset stays a description of tasks, so the same dataset is reusable across skill variants and across agents. ## Usage -A single `skills.path` field points at a directory of skill directories: +A single `skills.path` field points at a directory of skill directories (here `--no-serve` collects against servers already started with `gym env start`): ```bash -ng_collect_rollouts +agent_name=my_agent \ +gym eval run --no-serve +agent_name=my_agent \ +input_jsonl_fpath=data/tasks.jsonl \ +output_jsonl_fpath=results/rollouts_variant_a.jsonl \ +skills.path=skills/variant_a/