Skip to content

Commit 1895a2c

Browse files
bai-uipathclaude
andauthored
fix(codex): copy skills into .agents/skills instead of symlinking (#225)
Codex follows a symlinked skill to its source, finds the skills repo's plugin manifest and lists the skill as `<plugin>:<skill>`; models sometimes turn that into a nonexistent `.agents/skills/<plugin>/<skill>/` path, and the fallback search (`rg --files`, `find`) skips symlinks and finds nothing. Skills are now copied into `.agents/skills/` before each turn and removed after it (and on stop/kill), so criteria and run artifacts never see them. Skill dirs the workspace already ships are left alone. The codex skills sample task now counts SKILL.md files with `find` and checks the copies are gone before grading. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 3ffb2ef commit 1895a2c

11 files changed

Lines changed: 191 additions & 61 deletions

File tree

‎.claude/notes/agents.md‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,8 +717,15 @@ being hardcoded, so a plugin that relocates its skills keeps working.
717717
skips external *plugins*, not configured skill paths. An inherited value is appended to
718718
rather than clobbered, since the host may legitimately configure OpenCode the same way.
719719
- **Pi** passes each as `--skill <dir>`.
720-
- **Codex** symlinks (or copies, on Windows) each skill dir into `.agents/skills/`, which
721-
the CLI auto-discovers from the working directory upward.
720+
- **Codex** copies each skill dir into `.agents/skills/`, which the CLI auto-discovers from
721+
the working directory upward. Copies, not symlinks, for two reasons that compound. Codex
722+
resolves a symlinked skill to its source, finds the plugin manifest there and lists the
723+
skill as `<plugin>:<skill>`; models sometimes turn that into the nonexistent path
724+
`.agents/skills/<plugin>/<skill>/SKILL.md`. The search that follows (`rg --files`,
725+
`find`) skips symlinks and returns nothing, so the agent works without the skill. The
726+
copies are made before each turn and removed after it, the same concern that kept
727+
OpenCode out of the sandbox: criteria and run artifacts must see only what the agent
728+
left. Skill dirs the workspace already has are never touched.
722729
- **Antigravity** takes search paths natively via `skills_paths` — but those only drive
723730
DISCOVERY. The file-tool allowlist is `workspaces` alone, so the skill roots must appear
724731
there too, or the agent discovers a skill and every read of its `SKILL.md` is denied as

‎docs/TASK_DEFINITION_GUIDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1345,7 +1345,7 @@ Like `skill_triggered`, this criterion emits a `ClassificationCriterionResult`,
13451345

13461346
### `skill_triggered`
13471347

1348-
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).
1348+
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).
13491349

13501350
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`.
13511351

‎docs/agents/ANTIGRAVITY.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ Antigravity supports [Agent Skills](https://agentskills.io/specification)
127127
2. the runtime plugin directory passed to `start()`.
128128

129129
For each source, the harness is handed whichever of `<source>/skills` or `<source>`
130-
directly contains subdirectories with a `SKILL.md`. Unlike Codex — which symlinks
130+
directly contains subdirectories with a `SKILL.md`. Unlike Codex — which copies
131131
skills into `.agents/skills/` — Antigravity is given the search-path roots directly
132132
via the SDK's `skills_paths`, and those roots are also added to the harness
133133
`workspaces` allowlist (otherwise the agent would find a skill but be denied the

‎docs/agents/CODEX.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ CodexAgent supports SKILL.md files following the [Agent Skills open standard](ht
115115
1. **config.plugins** - Local plugins with `type: local` and `path` pointing to a skills directory
116116
2. **plugin_tools_dir** parameter - Runtime plugin directory passed to `start()`
117117

118-
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.
118+
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.
119119

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

‎docs/agents/HARNESS_PARITY.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ This field breaks it silently.
684684
claude-code hands the value to the SDK as a *plugin directory*, and a plugin's skills
685685
live at `<plugin>/skills/<name>/SKILL.md`. Point it at the directory that directly
686686
parents the skill directories and no skill loads. Codex
687-
(`codex_agent._setup_skills`) and Antigravity (`antigravity_agent._resolve_skills_paths`)
687+
(`codex_agent._collect_skills`) and Antigravity (`antigravity_agent._resolve_skills_paths`)
688688
both scan **both** layouts and take whichever actually holds a `<skill>/SKILL.md`.
689689

690690
† Pi uses the shared `_plugin_skill_dirs` resolver, whose bare-dir fallback resolves a

‎src/coder_eval/agents/codex_agent.py‎

Lines changed: 88 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,9 @@ def __init__(
854854
self.working_directory: Path | None = None
855855
self._env_path_prepend: list[str] = []
856856
self._login_shell_home: Path | None = None
857+
self._skill_dirs: list[Path] = []
858+
self._skill_copies: list[Path] = []
859+
self._skill_parent_dirs: list[Path] = []
857860
# _state / _iteration / _iteration_was_incremented / pending_turn lifecycle
858861
# bookkeeping lives on the Agent base class (shared defaults + helpers).
859862
self._log = PrefixedAdapter(logger, {"prefix": instance_name})
@@ -909,8 +912,7 @@ async def start(
909912
"CodexAgent: login_api_key failed — agent will fall back to env-based auth: %s", exc
910913
)
911914

912-
# Set up skills from plugin_tools_dir or plugins config
913-
self._setup_skills(plugin_tools_dir)
915+
self._collect_skills(plugin_tools_dir)
914916

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

959+
self._install_skills()
960+
try:
961+
return await self._communicate_turn(
962+
user_input,
963+
stream_callback=stream_callback,
964+
timeout=timeout,
965+
max_turns=max_turns,
966+
should_stop=should_stop,
967+
)
968+
finally:
969+
self._remove_skills()
970+
971+
async def _communicate_turn(
972+
self,
973+
user_input: str,
974+
*,
975+
stream_callback: StreamCallback | None,
976+
timeout: float | None,
977+
max_turns: int | None,
978+
should_stop: Callable[[], bool] | None,
979+
) -> TurnRecord:
957980
assert self.config.type is not None, "CodexAgent requires AgentConfig.type to be set before communicate()"
958981

959982
# Reset the pending slot + bump the iteration counter (shared lifecycle).
@@ -1092,6 +1115,7 @@ async def stop(self) -> None:
10921115
self._thread_usage_baseline = _ThreadTotals()
10931116
self._active_turn_handle = None
10941117
self._cleanup_login_shell_home()
1118+
self._remove_skills()
10951119
self._mark_stopped()
10961120

10971121
async def kill(self) -> None:
@@ -1108,6 +1132,7 @@ def kill_sync(self) -> None:
11081132
self._interrupt_active_turn()
11091133
self._close_client()
11101134
self._cleanup_login_shell_home()
1135+
self._remove_skills()
11111136

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

1150-
def _setup_skills(self, plugin_tools_dir: str | None) -> None:
1151-
"""Set up .agents/skills directory from plugins or plugin_tools_dir.
1175+
def _collect_skills(self, plugin_tools_dir: str | None) -> None:
1176+
"""Resolve the skill dirs that :meth:`_install_skills` copies in for each turn.
11521177
1153-
Codex auto-discovers skills in ``.agents/skills/``, scanned from the working
1154-
directory up to the repo root, so each source's skill dirs are symlinked
1155-
(or copied, on Windows) into it.
1178+
A source may hold skill dirs directly or be a plugin root whose skills live
1179+
one level deeper; both layouts are scanned, and the first source to provide
1180+
a skill name wins.
11561181
11571182
Rationale: .claude/notes/agents.md § Skills, per harness
11581183
"""
1159-
if not self.working_directory:
1160-
return
1184+
self._skill_dirs = []
11611185

11621186
skills_sources: list[Path] = []
11631187

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

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

1195-
# Create .agents/skills directory (Codex auto-discovery location)
1196-
agents_skills_dir = self.working_directory / ".agents" / "skills"
11971219
try:
1198-
agents_skills_dir.mkdir(parents=True, exist_ok=True)
1199-
1200-
# A source may hold skill dirs directly or be a plugin root whose
1201-
# skills live one level deeper. Scan both layouts.
12021220
for skills_source in skills_sources:
12031221
scan_dirs = [skills_source]
12041222
nested = skills_source / "skills"
12051223
if nested.is_dir():
12061224
scan_dirs.append(nested)
1207-
12081225
for scan_dir in scan_dirs:
12091226
for skill_dir in scan_dir.iterdir():
12101227
if not (skill_dir.is_dir() and (skill_dir / "SKILL.md").exists()):
12111228
continue
1212-
target = agents_skills_dir / skill_dir.name
1213-
if target.exists():
1214-
# Skip if already exists (first source wins)
1215-
continue
1229+
if all(d.name != skill_dir.name for d in self._skill_dirs):
1230+
self._skill_dirs.append(skill_dir)
1231+
except OSError as e:
1232+
self._log.warning(f"Failed to scan skills: {e}")
12161233

1217-
try:
1218-
# Try to create a symlink for efficiency
1219-
target.symlink_to(skill_dir)
1220-
self._log.debug(f"Linked skill: {skill_dir.name}")
1221-
except (OSError, NotImplementedError):
1222-
# Fall back to copying if symlink fails (Windows compatibility)
1223-
shutil.copytree(skill_dir, target, dirs_exist_ok=True)
1224-
self._log.debug(f"Copied skill: {skill_dir.name}")
1225-
1226-
linked = list(agents_skills_dir.iterdir())
1227-
if linked:
1228-
self._log.debug(f"Linked {len(linked)} skill(s) into {agents_skills_dir}")
1229-
else:
1230-
# Sources existed but held no SKILL.md, so codex runs with no
1231-
# skill context at all.
1232-
self._log.warning(
1233-
f"0 skills linked into {agents_skills_dir} despite "
1234-
+ f"{len(skills_sources)} plugin source(s): "
1235-
+ f"{[str(s) for s in skills_sources]}; "
1236-
+ "check the plugin path points at a skills repo root"
1237-
)
1234+
if self._skill_dirs:
1235+
self._log.debug(f"Found {len(self._skill_dirs)} skill(s): {[d.name for d in self._skill_dirs]}")
1236+
else:
1237+
# Sources existed but held no SKILL.md, so codex runs with no
1238+
# skill context at all.
1239+
self._log.warning(
1240+
f"0 skills found in {len(skills_sources)} plugin source(s): "
1241+
+ f"{[str(s) for s in skills_sources]}; "
1242+
+ "check the plugin path points at a skills repo root"
1243+
)
12381244

1245+
def _install_skills(self) -> None:
1246+
"""Copy the collected skills into ``.agents/skills/``, where Codex auto-discovers them.
1247+
1248+
Real copies, not symlinks. Codex resolves a symlinked skill to its source,
1249+
finds the plugin manifest there and lists it as ``<plugin>:<skill>``, which
1250+
models turn into a nonexistent ``<plugin>/<skill>/`` path; ``rg --files`` and
1251+
``find`` then skip the symlinks, so the search that follows finds nothing. A
1252+
skill dir already in the workspace is left alone and never removed.
1253+
1254+
Rationale: .claude/notes/agents.md § Skills, per harness
1255+
"""
1256+
if not (self.working_directory and self._skill_dirs):
1257+
return
1258+
agents_skills_dir = self.working_directory / ".agents" / "skills"
1259+
try:
1260+
for parent in (agents_skills_dir.parent, agents_skills_dir):
1261+
if not parent.exists():
1262+
parent.mkdir()
1263+
self._skill_parent_dirs.append(parent)
1264+
for skill_dir in self._skill_dirs:
1265+
target = agents_skills_dir / skill_dir.name
1266+
if target.exists() or target.is_symlink():
1267+
continue
1268+
self._skill_copies.append(target)
1269+
shutil.copytree(skill_dir, target, symlinks=True)
1270+
self._log.debug(f"Copied {len(self._skill_copies)} skill(s) into {agents_skills_dir}")
12391271
except Exception as e:
12401272
self._log.warning(f"Failed to set up skills: {e}")
12411273

1274+
def _remove_skills(self) -> None:
1275+
"""Remove what :meth:`_install_skills` created (best-effort, idempotent).
1276+
1277+
Runs at the end of every turn, so criteria and run artifacts see the workspace
1278+
the agent left, not the skill copies.
1279+
"""
1280+
copies, self._skill_copies = self._skill_copies, []
1281+
parents, self._skill_parent_dirs = self._skill_parent_dirs, []
1282+
for copy in copies:
1283+
shutil.rmtree(copy, ignore_errors=True)
1284+
for parent in reversed(parents):
1285+
with contextlib.suppress(OSError):
1286+
parent.rmdir()
1287+
12421288
@staticmethod
12431289
def _resolve_base_url() -> str | None:
12441290
"""Custom Codex endpoint base URL from CODEX_BASE_URL, or None."""

‎tasks/agents/codex_skills_test.yaml‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
task_id: codex_skills_test
2-
description: Test that CodexAgent properly sets up and discovers SKILL.md files
2+
description: Test that CodexAgent installs SKILL.md files for the turn and removes them before grading
33

44
tags:
55
- codex
@@ -15,16 +15,16 @@ agent:
1515
path: "$PLUGIN_PATH"
1616

1717
initial_prompt: |
18-
List all available skills in your environment by checking the .agents/skills directory.
19-
Count how many SKILL.md files are available.
18+
Run `find .agents/skills -name SKILL.md | wc -l` and write the number it prints, and
19+
nothing else, to skill_count.txt.
2020
2121
success_criteria:
2222
- type: run_command
23-
description: .agents/skills directory should exist
24-
command: test -d .agents/skills
23+
description: find should see at least one SKILL.md under .agents/skills
24+
command: grep -Eq '^[[:space:]]*[1-9][0-9]*[[:space:]]*$' skill_count.txt
2525
expected_exit_code: 0
2626

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

0 commit comments

Comments
 (0)