Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .claude/notes/agents.md
Original file line number Diff line number Diff line change
Expand Up @@ -717,8 +717,15 @@ being hardcoded, so a plugin that relocates its skills keeps working.
skips external *plugins*, not configured skill paths. An inherited value is appended to
rather than clobbered, since the host may legitimately configure OpenCode the same way.
- **Pi** passes each as `--skill <dir>`.
- **Codex** symlinks (or copies, on Windows) each skill dir into `.agents/skills/`, which
the CLI auto-discovers from the working directory upward.
- **Codex** copies each skill dir into `.agents/skills/`, which the CLI auto-discovers from
the working directory upward. Copies, not symlinks, for two reasons that compound. Codex
resolves a symlinked skill to its source, finds the plugin manifest there and lists the
skill as `<plugin>:<skill>`; models sometimes turn that into the nonexistent path
`.agents/skills/<plugin>/<skill>/SKILL.md`. The search that follows (`rg --files`,
`find`) skips symlinks and returns nothing, so the agent works without the skill. The
copies are made before each turn and removed after it, the same concern that kept
OpenCode out of the sandbox: criteria and run artifacts must see only what the agent
left. Skill dirs the workspace already has are never touched.
- **Antigravity** takes search paths natively via `skills_paths` — but those only drive
DISCOVERY. The file-tool allowlist is `workspaces` alone, so the skill roots must appear
there too, or the agent discovers a skill and every read of its `SKILL.md` is denied as
Expand Down
2 changes: 1 addition & 1 deletion docs/TASK_DEFINITION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ Like `skill_triggered`, this criterion emits a `ClassificationCriterionResult`,

### `skill_triggered`

Binary classifier: **did the agent engage the target skill during the run?** Agent-agnostic — scans the run's `turn_records` for either signal: Claude's explicit `Skill` tool call whose `skill` parameter matches `skill_name` (namespace prefixes like `plugin:skill` are stripped, so `skill_name: uipath-agents` matches `Skill(skill="uipath-coded-agents:uipath-agents")`), or — for an agent with no `Skill` tool, e.g. Codex — a command that reads the skill's files off disk (a parameter contains `skills/<skill_name>/`, matching both the repo path and the `.agents/skills/` symlink).
Binary classifier: **did the agent engage the target skill during the run?** Agent-agnostic — scans the run's `turn_records` for either signal: Claude's explicit `Skill` tool call whose `skill` parameter matches `skill_name` (namespace prefixes like `plugin:skill` are stripped, so `skill_name: uipath-agents` matches `Skill(skill="uipath-coded-agents:uipath-agents")`), or — for an agent with no `Skill` tool, e.g. Codex — a command that reads the skill's files off disk (a parameter contains `skills/<skill_name>/`, matching both the repo path and the `.agents/skills/` copy).

Observed label is `"yes"` when either signal is found, else `"no"`. Expected label is `"yes"` iff `expected_skill == skill_name`. **Binary scoring:** `1.0` when observed matches expected, else `0.0`.

Expand Down
2 changes: 1 addition & 1 deletion docs/agents/ANTIGRAVITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ Antigravity supports [Agent Skills](https://agentskills.io/specification)
2. the runtime plugin directory passed to `start()`.

For each source, the harness is handed whichever of `<source>/skills` or `<source>`
directly contains subdirectories with a `SKILL.md`. Unlike Codex — which symlinks
directly contains subdirectories with a `SKILL.md`. Unlike Codex — which copies
skills into `.agents/skills/` — Antigravity is given the search-path roots directly
via the SDK's `skills_paths`, and those roots are also added to the harness
`workspaces` allowlist (otherwise the agent would find a skill but be denied the
Expand Down
5 changes: 3 additions & 2 deletions docs/agents/CODEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ CodexAgent supports SKILL.md files following the [Agent Skills open standard](ht
1. **config.plugins** - Local plugins with `type: local` and `path` pointing to a skills directory
2. **plugin_tools_dir** parameter - Runtime plugin directory passed to `start()`

Skills are symlinked (or copied) to `.agents/skills/` where the Codex CLI auto-discovers them. Environment variables in plugin paths (`$VAR`, `${VAR}`) are expanded at runtime.
Skills are copied to `.agents/skills/` for each turn, where the Codex CLI auto-discovers them, and removed when the turn ends. Environment variables in plugin paths (`$VAR`, `${VAR}`) are expanded at runtime.

Example with environment variable:
```yaml
Expand Down Expand Up @@ -206,8 +206,9 @@ The agent sets up SKILL.md files (Agent Skills open standard) in `.agents/skills
1. Scans `config.plugins` for local plugins with `path` field
2. Checks `plugin_tools_dir` parameter passed to `start()`
3. Expands environment variables in paths (`$PLUGIN_PATH`, `${PLUGIN_PATH}`)
4. Symlinks skill directories (falls back to copying if symlink fails)
4. Copies each skill directory into `.agents/skills/` before every turn (real files, not symlinks), skipping any skill dir the workspace already has
5. Codex CLI auto-discovers skills in `.agents/skills/`
6. Removes the copies when the turn ends, so criteria and run artifacts never see them

### Async Integration

Expand Down
2 changes: 1 addition & 1 deletion docs/agents/HARNESS_PARITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ This field breaks it silently.
claude-code hands the value to the SDK as a *plugin directory*, and a plugin's skills
live at `<plugin>/skills/<name>/SKILL.md`. Point it at the directory that directly
parents the skill directories and no skill loads. Codex
(`codex_agent._setup_skills`) and Antigravity (`antigravity_agent._resolve_skills_paths`)
(`codex_agent._collect_skills`) and Antigravity (`antigravity_agent._resolve_skills_paths`)
both scan **both** layouts and take whichever actually holds a `<skill>/SKILL.md`.

† Pi uses the shared `_plugin_skill_dirs` resolver, whose bare-dir fallback resolves a
Expand Down
130 changes: 88 additions & 42 deletions src/coder_eval/agents/codex_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,9 @@ def __init__(
self.working_directory: Path | None = None
self._env_path_prepend: list[str] = []
self._login_shell_home: Path | None = None
self._skill_dirs: list[Path] = []
self._skill_copies: list[Path] = []
self._skill_parent_dirs: list[Path] = []
# _state / _iteration / _iteration_was_incremented / pending_turn lifecycle
# bookkeeping lives on the Agent base class (shared defaults + helpers).
self._log = PrefixedAdapter(logger, {"prefix": instance_name})
Expand Down Expand Up @@ -909,8 +912,7 @@ async def start(
"CodexAgent: login_api_key failed — agent will fall back to env-based auth: %s", exc
)

# Set up skills from plugin_tools_dir or plugins config
self._setup_skills(plugin_tools_dir)
self._collect_skills(plugin_tools_dir)

# Log permission and tool configuration
self._log_config_enforcement()
Expand Down Expand Up @@ -954,6 +956,27 @@ async def communicate(
if not self.working_directory or not self.codex_client:
raise RuntimeError("Agent not started. Call start() first.")

self._install_skills()
try:
return await self._communicate_turn(
user_input,
stream_callback=stream_callback,
timeout=timeout,
max_turns=max_turns,
should_stop=should_stop,
)
finally:
self._remove_skills()

async def _communicate_turn(
self,
user_input: str,
*,
stream_callback: StreamCallback | None,
timeout: float | None,
max_turns: int | None,
should_stop: Callable[[], bool] | None,
) -> TurnRecord:
assert self.config.type is not None, "CodexAgent requires AgentConfig.type to be set before communicate()"

# Reset the pending slot + bump the iteration counter (shared lifecycle).
Expand Down Expand Up @@ -1092,6 +1115,7 @@ async def stop(self) -> None:
self._thread_usage_baseline = _ThreadTotals()
self._active_turn_handle = None
self._cleanup_login_shell_home()
self._remove_skills()
self._mark_stopped()

async def kill(self) -> None:
Expand All @@ -1108,6 +1132,7 @@ def kill_sync(self) -> None:
self._interrupt_active_turn()
self._close_client()
self._cleanup_login_shell_home()
self._remove_skills()

def _interrupt_active_turn(self) -> None:
"""Interrupt the in-flight Codex turn, if any (best-effort, idempotent)."""
Expand Down Expand Up @@ -1147,17 +1172,16 @@ def get_environment_info(self) -> dict[str, Any]:
"codex_model_is_deployment": True,
}

def _setup_skills(self, plugin_tools_dir: str | None) -> None:
"""Set up .agents/skills directory from plugins or plugin_tools_dir.
def _collect_skills(self, plugin_tools_dir: str | None) -> None:
"""Resolve the skill dirs that :meth:`_install_skills` copies in for each turn.

Codex auto-discovers skills in ``.agents/skills/``, scanned from the working
directory up to the repo root, so each source's skill dirs are symlinked
(or copied, on Windows) into it.
A source may hold skill dirs directly or be a plugin root whose skills live
one level deeper; both layouts are scanned, and the first source to provide
a skill name wins.

Rationale: .claude/notes/agents.md § Skills, per harness
"""
if not self.working_directory:
return
self._skill_dirs = []

skills_sources: list[Path] = []

Expand All @@ -1179,7 +1203,7 @@ def _setup_skills(self, plugin_tools_dir: str | None) -> None:
hint = "env var likely unset" if "$" in expanded_path else "path does not exist"
self._log.warning(
f"Plugin skills path did not resolve: {path_str!r} "
+ f"→ {expanded_path!r} ({hint}); no skills linked from it"
+ f"→ {expanded_path!r} ({hint}); no skills taken from it"
)

# Also check plugin_tools_dir parameter
Expand All @@ -1192,53 +1216,75 @@ def _setup_skills(self, plugin_tools_dir: str | None) -> None:
if not skills_sources:
return

# Create .agents/skills directory (Codex auto-discovery location)
agents_skills_dir = self.working_directory / ".agents" / "skills"
try:
agents_skills_dir.mkdir(parents=True, exist_ok=True)

# A source may hold skill dirs directly or be a plugin root whose
# skills live one level deeper. Scan both layouts.
for skills_source in skills_sources:
scan_dirs = [skills_source]
nested = skills_source / "skills"
if nested.is_dir():
scan_dirs.append(nested)

for scan_dir in scan_dirs:
for skill_dir in scan_dir.iterdir():
if not (skill_dir.is_dir() and (skill_dir / "SKILL.md").exists()):
continue
target = agents_skills_dir / skill_dir.name
if target.exists():
# Skip if already exists (first source wins)
continue
if all(d.name != skill_dir.name for d in self._skill_dirs):
self._skill_dirs.append(skill_dir)
except OSError as e:
self._log.warning(f"Failed to scan skills: {e}")

try:
# Try to create a symlink for efficiency
target.symlink_to(skill_dir)
self._log.debug(f"Linked skill: {skill_dir.name}")
except (OSError, NotImplementedError):
# Fall back to copying if symlink fails (Windows compatibility)
shutil.copytree(skill_dir, target, dirs_exist_ok=True)
self._log.debug(f"Copied skill: {skill_dir.name}")

linked = list(agents_skills_dir.iterdir())
if linked:
self._log.debug(f"Linked {len(linked)} skill(s) into {agents_skills_dir}")
else:
# Sources existed but held no SKILL.md, so codex runs with no
# skill context at all.
self._log.warning(
f"0 skills linked into {agents_skills_dir} despite "
+ f"{len(skills_sources)} plugin source(s): "
+ f"{[str(s) for s in skills_sources]}; "
+ "check the plugin path points at a skills repo root"
)
if self._skill_dirs:
self._log.debug(f"Found {len(self._skill_dirs)} skill(s): {[d.name for d in self._skill_dirs]}")
else:
# Sources existed but held no SKILL.md, so codex runs with no
# skill context at all.
self._log.warning(
f"0 skills found in {len(skills_sources)} plugin source(s): "
+ f"{[str(s) for s in skills_sources]}; "
+ "check the plugin path points at a skills repo root"
)

def _install_skills(self) -> None:
"""Copy the collected skills into ``.agents/skills/``, where Codex auto-discovers them.

Real copies, not symlinks. Codex resolves a symlinked skill to its source,
finds the plugin manifest there and lists it as ``<plugin>:<skill>``, which
models turn into a nonexistent ``<plugin>/<skill>/`` path; ``rg --files`` and
``find`` then skip the symlinks, so the search that follows finds nothing. A
skill dir already in the workspace is left alone and never removed.

Rationale: .claude/notes/agents.md § Skills, per harness
"""
if not (self.working_directory and self._skill_dirs):
return
agents_skills_dir = self.working_directory / ".agents" / "skills"
try:
for parent in (agents_skills_dir.parent, agents_skills_dir):
if not parent.exists():
parent.mkdir()
self._skill_parent_dirs.append(parent)
for skill_dir in self._skill_dirs:
target = agents_skills_dir / skill_dir.name
if target.exists() or target.is_symlink():
continue
self._skill_copies.append(target)
shutil.copytree(skill_dir, target, symlinks=True)
self._log.debug(f"Copied {len(self._skill_copies)} skill(s) into {agents_skills_dir}")
except Exception as e:
self._log.warning(f"Failed to set up skills: {e}")

def _remove_skills(self) -> None:
"""Remove what :meth:`_install_skills` created (best-effort, idempotent).

Runs at the end of every turn, so criteria and run artifacts see the workspace
the agent left, not the skill copies.
"""
copies, self._skill_copies = self._skill_copies, []
parents, self._skill_parent_dirs = self._skill_parent_dirs, []
for copy in copies:
shutil.rmtree(copy, ignore_errors=True)
for parent in reversed(parents):
with contextlib.suppress(OSError):
parent.rmdir()

@staticmethod
def _resolve_base_url() -> str | None:
"""Custom Codex endpoint base URL from CODEX_BASE_URL, or None."""
Expand Down
14 changes: 7 additions & 7 deletions tasks/agents/codex_skills_test.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
task_id: codex_skills_test
description: Test that CodexAgent properly sets up and discovers SKILL.md files
description: Test that CodexAgent installs SKILL.md files for the turn and removes them before grading

tags:
- codex
Expand All @@ -15,16 +15,16 @@ agent:
path: "$PLUGIN_PATH"

initial_prompt: |
List all available skills in your environment by checking the .agents/skills directory.
Count how many SKILL.md files are available.
Run `find .agents/skills -name SKILL.md | wc -l` and write the number it prints, and
nothing else, to skill_count.txt.

success_criteria:
- type: run_command
description: .agents/skills directory should exist
command: test -d .agents/skills
description: find should see at least one SKILL.md under .agents/skills
command: grep -Eq '^[[:space:]]*[1-9][0-9]*[[:space:]]*$' skill_count.txt
expected_exit_code: 0

- type: run_command
description: Should have at least one SKILL.md file
command: find .agents/skills -name "SKILL.md" | wc -l | grep -v "^0$"
description: The skill copies are gone by grading time
command: test ! -e .agents
expected_exit_code: 0
Loading
Loading