Skip to content

Commit 2e0a707

Browse files
author
codejunkie99
committed
review: address codex-adapter review findings
Cross-model review (Claude + Codex adversarial) flagged real gaps in the codex adapter. Fixes here: 1. AGENTS.md collision was "skip if exists" — silent-success when the user already has an Aider/Amp/Cline/old-codex AGENTS.md. Mirror the openclaw pattern: if existing AGENTS.md references .agent/, leave alone; if not, print a mergeable snippet and don't overwrite. Same change in install.sh and install.ps1. 2. .agents/skills/ sync was destructive (cp -R overlay) and non-healing (deleted skills lingered forever). Switch to rsync --delete when available, fall back to rm+cp, so the .agents/skills mirror stays in sync with the .agent/skills source of truth. 3. install.ps1 used `Test-Path` then `Copy-Item ... -Force`, which on PowerShell 5.1 (Windows default) writes through a symlink into the target — silently mutating .agent/skills via the link. Detect ReparsePoint via Get-Item.Attributes BEFORE Remove-Item; use .NET Directory.Delete($path, false) on links so only the link is removed, never the target. 4. adapters/codex/AGENTS.md: add a Windows note about python vs python3 (stock Windows only ships `python` on PATH). 5. Citation: link OpenAI's https://developers.openai.com/codex/skills in the adapter docs to make the .agents/skills/ contract explicit. Smoke tested: fresh install, re-run with real-dir mirror + orphan skill (orphan deleted via rsync), AGENTS.md collision branches both covered.
1 parent fb7b898 commit 2e0a707

3 files changed

Lines changed: 87 additions & 14 deletions

File tree

‎adapters/codex/AGENTS.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,23 @@
33
Codex reads `AGENTS.md` before doing any work. This file points it at
44
the portable brain in `.agent/`.
55

6+
> **Python invocation**: examples below use `python3`. On stock Windows
7+
> only `python` is on PATH; use whichever resolves on your system.
8+
69
## Startup (read in order)
710
1. `.agent/AGENTS.md` — the map
811
2. `.agent/memory/personal/PREFERENCES.md` — user conventions
912
3. `.agent/memory/semantic/LESSONS.md` — distilled lessons
1013
4. `.agent/protocols/permissions.md` — hard rules
1114

1215
## Skills
13-
Codex scans `.agents/skills/` for repository-scoped skills. The install
14-
script symlinks or copies `.agents/skills` from `.agent/skills` so the
15-
portable brain remains the one source of truth. Load a full `SKILL.md`
16-
only when its triggers match the task (progressive disclosure).
16+
Codex scans `.agents/skills/` for repository-scoped skills (per
17+
[OpenAI Codex docs](https://developers.openai.com/codex/skills)). The
18+
install script symlinks or syncs `.agents/skills` from `.agent/skills`
19+
so the portable brain remains the one source of truth. Load a full
20+
`SKILL.md` only when its triggers match the task (progressive
21+
disclosure). Edit skills in `.agent/skills/` — `.agents/skills/` is a
22+
mirror and re-running the installer will sync it back.
1723

1824
## Recall before non-trivial tasks
1925
For deploy / ship / migration / schema / timestamp / date / failing test /

‎install.ps1‎

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,22 +73,65 @@ switch ($Adapter) {
7373
Copy-Item (Join-Path $Src 'AGENTS.md') (Join-Path $TargetDir 'AGENTS.md') -Force
7474
}
7575
'codex' {
76+
# Mirror install.sh: openclaw-style merge-or-alert on existing AGENTS.md.
7677
$agentsMd = Join-Path $TargetDir 'AGENTS.md'
7778
if (Test-Path $agentsMd -PathType Leaf) {
78-
Write-Host " ~ $agentsMd already exists — skipping (codex reads whatever is there)"
79+
$existing = Get-Content -Path $agentsMd -Raw -ErrorAction SilentlyContinue
80+
if ($existing -match '\.agent/') {
81+
Write-Host " ~ AGENTS.md already references .agent/ — leaving alone"
82+
} else {
83+
Write-Host " ! AGENTS.md exists but does not reference .agent/; not overwriting."
84+
Write-Host " merge this block into your AGENTS.md to wire the brain:"
85+
Write-Host " ---8<---"
86+
Get-Content -Path (Join-Path $Src 'AGENTS.md') | ForEach-Object { Write-Host " $_" }
87+
Write-Host " --->8---"
88+
}
7989
} else {
8090
Copy-Item (Join-Path $Src 'AGENTS.md') $agentsMd -Force
8191
Write-Host " + AGENTS.md"
8292
}
8393

94+
# Codex scans .agents/skills/ — keep the portable brain authoritative.
8495
$agentsDir = Join-Path $TargetDir '.agents'
8596
New-Item -ItemType Directory -Path $agentsDir -Force | Out-Null
8697
$skillsSrc = Join-Path $TargetAgent 'skills'
8798
$skillsDst = Join-Path $agentsDir 'skills'
8899

89-
if (Test-Path $skillsDst) {
90-
Copy-Item -Path (Join-Path $skillsSrc '*') -Destination $skillsDst -Recurse -Force
91-
Write-Host " ~ merged .agent/skills into existing .agents/skills"
100+
# Detect symlink/junction BEFORE Remove-Item: on PowerShell 5.1
101+
# `Remove-Item -Recurse` on a symlink can delete the target's
102+
# contents. Use IsLink detection + .NET Delete (or repoint).
103+
$skillsDstItem = Get-Item -LiteralPath $skillsDst -Force -ErrorAction SilentlyContinue
104+
$isLink = $false
105+
if ($skillsDstItem) {
106+
$isLink = ($skillsDstItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -eq [System.IO.FileAttributes]::ReparsePoint
107+
}
108+
109+
if ($skillsDstItem -and $isLink) {
110+
# Existing link: delete the link only (NOT its target), then re-create.
111+
try {
112+
[System.IO.Directory]::Delete($skillsDst, $false)
113+
} catch {
114+
# Some Windows configurations require File.Delete for file-style links.
115+
[System.IO.File]::Delete($skillsDst)
116+
}
117+
try {
118+
New-Item -ItemType SymbolicLink -Path $skillsDst -Target $skillsSrc -ErrorAction Stop | Out-Null
119+
Write-Host " + .agents/skills -> $skillsSrc (relinked)"
120+
} catch {
121+
Copy-Item -Path $skillsSrc -Destination $skillsDst -Recurse
122+
Write-Host " + .agents/skills (copy; symlink not supported here)"
123+
}
124+
} elseif ($skillsDstItem) {
125+
# Real directory: sync with delete-orphans by replacing it whole.
126+
# Removing a real directory with -Recurse is safe; only links are dangerous.
127+
Remove-Item -LiteralPath $skillsDst -Recurse -Force
128+
try {
129+
New-Item -ItemType SymbolicLink -Path $skillsDst -Target $skillsSrc -ErrorAction Stop | Out-Null
130+
Write-Host " + .agents/skills -> $skillsSrc (replaced stale copy)"
131+
} catch {
132+
Copy-Item -Path $skillsSrc -Destination $skillsDst -Recurse
133+
Write-Host " ~ replaced .agents/skills with current .agent/skills (no symlink)"
134+
}
92135
} else {
93136
try {
94137
New-Item -ItemType SymbolicLink -Path $skillsDst -Target $skillsSrc -ErrorAction Stop | Out-Null

‎install.sh‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,27 +86,51 @@ case "$ADAPTER" in
8686
fi
8787
;;
8888
codex)
89-
# codex, pi, hermes, and opencode can all read the same AGENTS.md
89+
# codex reads AGENTS.md (like pi, hermes, opencode). Many other tools
90+
# also write AGENTS.md (aider, amp, cline, existing codex setups), so
91+
# we follow the openclaw pattern: merge-or-alert, never blind overwrite,
92+
# never blind skip.
9093
if [[ -f "$TARGET/AGENTS.md" ]]; then
91-
echo " ~ $TARGET/AGENTS.md already exists — skipping (codex reads whatever is there)"
94+
if grep -q '\.agent/' "$TARGET/AGENTS.md" 2>/dev/null; then
95+
echo " ~ AGENTS.md already references .agent/ — leaving alone"
96+
else
97+
echo " ! AGENTS.md exists but does not reference .agent/; not overwriting."
98+
echo " merge this block into your AGENTS.md to wire the brain:"
99+
echo " ---8<---"
100+
sed 's/^/ /' "$SRC/AGENTS.md"
101+
echo " --->8---"
102+
fi
92103
else
93104
cp "$SRC/AGENTS.md" "$TARGET/AGENTS.md"
94105
echo " + AGENTS.md"
95106
fi
107+
108+
# Codex scans .agents/skills/ (plural) for repo-scoped skills — per
109+
# OpenAI docs https://developers.openai.com/codex/skills. Keep the
110+
# portable brain authoritative: .agents/skills mirrors .agent/skills.
96111
mkdir -p "$TARGET/.agents"
97112
SKILLS_SRC="$(cd "$TARGET/.agent/skills" && pwd)"
98113
SKILLS_DEST="$TARGET/.agents/skills"
99114
if [[ -L "$SKILLS_DEST" ]]; then
115+
# Existing symlink: repoint at current .agent/skills (cheap, safe)
100116
ln -sfn "$SKILLS_SRC" "$SKILLS_DEST"
101117
echo " + .agents/skills -> $SKILLS_SRC"
102118
elif [[ -d "$SKILLS_DEST" ]]; then
103-
cp -R "$SKILLS_SRC/." "$SKILLS_DEST/"
104-
echo " ~ merged .agent/skills into existing .agents/skills"
119+
# Real directory from a prior copy-fallback install: sync with
120+
# delete-orphans so removed/renamed skills don't linger. Use rsync
121+
# if available, otherwise rm+cp as a safe-but-blunt replacement.
122+
if command -v rsync >/dev/null 2>&1; then
123+
rsync -a --delete "$SKILLS_SRC/" "$SKILLS_DEST/"
124+
echo " ~ synced .agent/skills → .agents/skills (rsync --delete)"
125+
else
126+
rm -rf "$SKILLS_DEST"
127+
cp -R "$SKILLS_SRC" "$SKILLS_DEST"
128+
echo " ~ replaced .agents/skills with current .agent/skills (no rsync)"
129+
fi
105130
elif ln -sfn "$SKILLS_SRC" "$SKILLS_DEST" 2>/dev/null; then
106131
echo " + .agents/skills -> $SKILLS_SRC"
107132
else
108-
mkdir -p "$SKILLS_DEST"
109-
cp -R "$SKILLS_SRC/." "$SKILLS_DEST/"
133+
cp -R "$SKILLS_SRC" "$SKILLS_DEST"
110134
echo " + .agents/skills (copy; symlink not supported here)"
111135
fi
112136
;;

0 commit comments

Comments
 (0)