Add codex adapter - #16
Merged
Merged
Conversation
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.
Owner
|
Hey @hovhannest — ran a cross-model review (Claude + Codex adversarial) on this and found a few things worth addressing. Rather than bounce a review back at you, I pushed a follow-up commit ( What changed in the follow-up:
One clarification: the initial Claude review flagged Tested: fresh install, re-run with real-dir mirror + orphan skill (orphan deleted via rsync), AGENTS.md collision branches both covered. Full review notes on request. |
This was referenced Apr 23, 2026
# Conflicts: # README.md # install.ps1
codejunkie99
pushed a commit
to hovhannest/agentic-stack
that referenced
this pull request
Apr 23, 2026
Two pre-existing infrastructure bugs flagged during the PR codejunkie99#17 cross-model review, fixed here against master because they predate that PR and affect every harness. ## 1. Concurrent writes to AGENT_LEARNINGS.jsonl post_execution.py and on_failure.py both did plain `open(EPISODIC, "a")` → `f.write(json.dumps(entry) + "\n")`. POSIX O_APPEND makes single `write(2)` calls atomic only up to PIPE_BUF (4 KB on Linux/macOS). In practice most entries stay under that ceiling and the unlocked code never corrupts, but the `reflection` field is uncapped in log_execution and can easily exceed 4 KB on high-importance failure logs. Every downstream reader (auto_dream.py, cluster.py, context_budget.py, show.py) skips `json.JSONDecodeError` lines silently — so one over-PIPE_BUF interleave = one episodic entry gone with no signal. Fix: new `_episodic_io.append_jsonl()` helper that opens in append- binary mode (no Python text-mode buffering quirks) and wraps the write in `fcntl.flock(LOCK_EX)`. Shared by both writers. On platforms without fcntl (native Windows Python) behavior falls back to the pre-fix unlocked append; WSL, git-bash/Cygwin, macOS, Linux all have fcntl. Verified: 40 concurrent writers × 500 entries × 2 KB reflection each → 20,000 parseable lines, zero corruption. ## 2. pi install.sh silently leaves stale skills on re-install `ln -sfn src dest` where `dest` is a REAL directory (e.g. from an earlier copy-fallback install) silently creates `dest/<basename-of-src>` INSIDE the dir and exits 0. The existing `if ln -sfn; then` branch took the success path, the `rm -rf + cp` fallback never ran, and orphans stuck around forever. Verified on macOS, confirmed the symlink-inside-dir behavior. Fix: check `-L` (symlink) and `-d` (real dir) explicitly before calling `ln -sfn`, mirror the pattern used by the codex adapter (PR codejunkie99#16 follow-up). Existing symlink → cheap repoint. Real directory → rsync --delete when available, rm+cp otherwise. Non-existent → symlink or copy fallback. Same three-branch shape, no more silent wrong behavior. Verified: re-install after orphan-skill was added to a real-dir `.pi/skills` → rsync --delete removes the orphan.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a Codex adapter for agentic-stack so the portable .agent/ brain can be used from Codex with the same lightweight
integration style as the existing harness adapters.
What changed
Adapter behavior
style used for non-hook harnesses
Notes
setups
manual memory tooling
Validation
Shorter version if you want a tighter PR body:
Summary
Add a Codex adapter for
agentic-stack.What changed
adapters/codex/AGENTS.mdadapters/codex/README.mddocs/per-harness/codex.mdcodexintoinstall.shandinstall.ps1Notes
The adapter uses root
AGENTS.mdplus.agents/skills/to expose the portable.agent/brain to Codex. Itintentionally does not install hooks and instead follows the existing manual
recall.py/memory_reflect.pypatternused by lighter integrations.