feat(codex): native plugin install manifests + agents-only converter#616
feat(codex): native plugin install manifests + agents-only converter#616
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 176a9407e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…een CE versions cleanup --target codex previously only checked names on the historical allow-list (EXTRA_LEGACY_ARTIFACTS_BY_PLUGIN). That catches renamed and removed skills/prompts but misses artifacts whose *emission format* changed between CE versions. The concrete case: CE previously emitted agents as generated skills under skills/<plugin>/<agent-name>/ and is now emitting them as TOML custom agents under agents/<plugin>/<name>.toml. Those stale agent-as-skill directories weren't on the historical allow- list (the names are CURRENT agent names, just in the wrong location) so cleanup left them in place. A subsequent install would self-clean via the managed-install manifest, but cleanup run alone left skills double-registered. Fix: cleanupCodex now reads the Codex install manifest and migrates any manifest-listed skills/prompts/agents that are not in the current bundle to legacy-backup. This gives cleanup parity with install-time cleanup — running cleanup alone completes the migration, no followup install required. readInstallManifest in src/targets/codex.ts was private. Exported as readCodexInstallManifest for the cleanup path. Regression test: a fake prior install with three current-named agents installed as namespaced skills + a stale prompt + a survivor skill, verified cleanup migrates the stale three + prompt while preserving the current-named skill that's still in the bundle. Surfaced during Unit 7 empirical verification of #616 (Codex native plugin install). Without this fix, users migrating from a pre-TOML- agents CE version would silently accumulate duplicate skill registrations unless they happened to run `install` after `cleanup`.
176a940 to
ec2e1c0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec2e1c057c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…Cursor
Adds .codex-plugin/plugin.json for compound-engineering and coding-tutor
plus a top-level .agents/plugins/marketplace.json so Codex's native
plugin install flow (`codex plugin marketplace add ...` +
`codex plugin install ...`) can register CE's skills directly, without
the Bun converter.
The Claude-format manifests stay authoritative for name/version/description;
Codex manifests hand-author the shared fields + Codex-specific interface
block (displayName, shortDescription, longDescription, category,
capabilities, defaultPrompt) and declare skills: "./skills/" so Codex's
skill discovery finds the existing flat skills/<name>/SKILL.md layout.
Schema verified against the openai-curated marketplace cached locally:
- .agents/plugins/marketplace.json uses nested source: { source: "local", path }
which works for both bundled and remote-cloned marketplaces
- plugin.json path resolves skills: "./skills/" relative to the plugin
directory, matching the Vercel and GitHub reference plugins
Agents and slash commands are NOT declarable in Codex's plugin format
per the spec and our empirical test. The existing Bun converter
(bunx @every-env/compound-plugin install ... --to codex) remains the
agent install path; README documents the two-step flow.
Extends the plugins/compound-engineering and plugins/coding-tutor package entries in .github/release-please-config.json with a third extra-files entry targeting .codex-plugin/plugin.json — parallel to the existing .claude-plugin/plugin.json and .cursor-plugin/plugin.json entries. release-please will now bump all three formats in lockstep on each release.
…ruth Extends syncReleaseMetadata with cross-checks for the Codex manifest surface. Mirrors the existing Claude <-> Cursor validation pattern, with one deliberate difference: version drift on .codex-plugin/plugin.json is detect-only, never auto-corrected on write: true. release-please owns version bumps via the extra-files entries in .github/release-please-config.json, and having syncReleaseMetadata also write would create a second authority for the same field. Only description drift is auto-corrected. New validation failure modes: - .codex-plugin/plugin.json missing when .claude-plugin/plugin.json exists (structural error) - name mismatch between paired plugin.json files (structural error) - skills: "./skills/" declared but target directory does not exist (structural error) - .agents/plugins/marketplace.json plugin list differs from .claude-plugin/marketplace.json — either direction (structural error) syncReleaseMetadata's return shape now includes errors: string[] alongside updates: FileUpdate[]. scripts/release/validate.ts surfaces these as "Release metadata structural errors" so CI fails with a specific remediation path. Tests cover the happy path plus each structural error mode and the write-mode behavior (description auto-corrected, version preserved).
Adds a ### Codex section between Cursor and the experimental-Bun list, matching the native-install pattern used by Claude Code and Cursor. Documents: - codex plugin marketplace add + codex plugin install commands for skills registration - The followup bunx ... --to codex step required for custom agents, with a clear explanation of why (Codex plugin format does not register custom agents or slash commands per spec) - cleanup --target codex pointer for users migrating from the Bun-only install - coding-tutor-specific note that its slash commands still require the Bun converter The existing "OpenCode, Codex, ... (experimental)" section still mentions Codex; that list gets pruned in a followup when PR #609 lands, since #609 restructures the experimental list and removes Codex from it.
…ketplace parity Adds Codex entries to the Versioning Requirements Contributor Rules and Pre-Commit Checklist, parallel to the existing Claude and Cursor entries. New .codex-plugin/plugin.json and .agents/plugins/marketplace.json are release-owned; hand-bumping them is forbidden (release-please owns version writes, validator catches content drift). Also adds an "Adding a New Plugin to This Repo" subsection under Adding Components, listing the three-marketplace parity checklist a contributor must walk when introducing a new plugin alongside compound-engineering and coding-tutor. The validator enforces every rule the doc states — a missed file is a CI failure, not a silent gap.
…tive install The Bun converter's `--to codex` default previously emitted skills, commands, and agents together. With Codex's native plugin install now handling skills (via the .codex-plugin/plugin.json manifest landed in an earlier commit), running both flows back-to-back double-registered skills — once from native install, once from the converter. New default: convertClaudeToCodex emits ONLY the agent conversions. skillDirs, prompts, command-skills, and mcpServers are suppressed so native install is the sole source for those artifact types. This makes the two-step flow additive: 1. codex plugin install compound-engineering # skills 2. bunx @every-env/compound-plugin install ... # agents (the gap) For users who cannot run native plugin install (offline, older Codex, custom workflow), --include-skills opts back into the full bundle. The flag is expected to be removed once Codex's native plugin spec supports custom agents — at which point the entire `--to codex` converter path becomes obsolete. Agent bodies still get the Claude -> Codex content transform in agents-only mode, so Task(...) and slash references in agent bodies rewrite against the skill graph that native install registers. The legacy sync command (src/sync/commands.ts) pins codexIncludeSkills: true — sync is not paired with native plugin install and must continue to emit the full bundle. Tests cover both modes: agents-only default (no skills, no prompts, just agent conversions), --include-skills full bundle, and the zero-agents plugin case (empty bundle, no possibility of duplicate skill install).
…ries Revises the Codex native manifests plan to reflect a mid-implementation correction: the Bun converter's --to codex default is now agents-only, with an --include-skills opt-in for standalone installs. The original plan framed agents as "out of scope," which missed that the two-step flow only works cleanly when the converter's default complements native install rather than duplicating it. Also flags that the entire --to codex converter path is expected to be deprecated once Codex's native plugin spec supports custom agents.
…collapsed note The previous Codex install section gave the standalone --include-skills path almost equal weight to the native flow, even though it's really an escape hatch. Rewrite to: - Open with the two-step flow as the single recommended path, both commands in one code block with inline comments explaining each - State plainly that both steps are needed (native = skills, followup = agents) and why (Codex spec doesn't register custom agents yet) - Move the --include-skills instructions into a <details> block so they're available but clearly secondary - Simplify the "when this goes away" framing
Post-rebase fix: cleanupCodex and cleanupCodexSharedAgents call convertClaudeToCodex to determine the current artifact set. With Unit 9's agents-only default, the bundle returns empty skills/prompts/generatedSkills — which made cleanup treat every existing skill directory as "not current" and try to remove it (including the live `codex/skills/<plugin>/<skill>/` trees). Pin codexIncludeSkills:true on both cleanup converter calls so cleanup sees the full bundle and makes correct current-vs-legacy decisions. Also update the `install --to all` test assertion: with agents-only default, the codex target writes agent TOML files (under ~/.codex/agents/<plugin>/) rather than skills (which native plugin install handles). Assert agent existence and skill non-existence to match the new contract.
Codex's plugin CLI only has `marketplace add | upgrade | remove` — there's no `codex plugin install <name>` subcommand. Plugins from an added marketplace are enabled via the interactive `/plugins` TUI inside Codex, which writes the same config.toml entry the CLI would produce if that subcommand existed. Empirically verified (Unit 7 of the plan): marketplace add + agents-only Bun step + /plugins TUI install + Codex restart = working install with skills discoverable and delegating skills finding their agents. Also update the standalone-install disclosure block to reference "marketplace add" rather than the non-existent "plugin install."
ec2e1c0 to
26d12bb
Compare
|
Thanks @tmchow given the recent shenanigans by Anthropic, I was really wondering whether we could hedge on Codex while keeping our beloved compound engineering workflow. Will spend a few checking out this. |
…st field (#616) Two fixes from PR #616 review: - Agents-only install no longer nukes active Codex skills. When codexIncludeSkills is false (the default), convertClaudeToCodex now records current skill names in a new externallyManagedSkillNames field on CodexBundle. writeCodexBundle unions those names into currentSkills so cleanupLegacyAgentSkillDirs stops treating them as legacy. Without this, re-running `install --to codex` on a system where Codex's native /plugins install owns the actual skill contents would move them into legacy-backup. Skill contents are still not copied — only the ownership metadata travels with the bundle. - Codex parity check treats `skills` as a required field. A .codex-plugin/plugin.json missing the skills declaration now fails release:validate with an error naming the plugin and missing field, instead of silently passing. The existing directory-existence check still runs when the field is present.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 503f31f4fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…t rhythm The prior layout put steps 1-2 in a bash block and step 3 in prose because the TUI interaction isn't a shell command. Visually jarring. Restructured as a numbered list where each step owns its own code block (or inline prose for the TUI step), so the reader sees a consistent 1->2->3 flow.
💡 Codex Reviewcompound-engineering-plugin/src/targets/codex.ts Lines 260 to 262 in e2e0184 This loop unconditionally moves every allow-listed
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Install-time prompt cleanup was moving any allow-listed filename under ~/.codex/prompts to legacy-backup without checking that CE wrote it. A user-authored ce-plan.md would get swept into backup on every install --to codex run with pluginName set. Restored the body + frontmatter ownership check the reviewer flagged as missing. New classifyCodexLegacyPromptOwnership helper in legacy-cleanup.ts returns ce-owned / foreign / unknown, reusing the two-signal fingerprint (body instruction boilerplate + frontmatter description against known-shipped CE descriptions) from the existing cleanupStalePrompts path. Applied at both the install-time path (cleanupKnownLegacyCodexArtifacts in codex.ts) and the standalone cleanup path (cleanupCodex in commands/cleanup.ts) for parity. Retired wrappers like reproduce-bug.md still clean up; real collision vectors like ce-plan.md are preserved.
`isOutdated` means the diff hunk shifted, not that the reviewer's concern was addressed. Filtering on it dropped unresolved threads whose surrounding lines happened to move, silently hiding valid feedback (e.g., PR #616 comment on src/targets/codex.ts:267). Filter on `isResolved` alone and surface `isOutdated` through dispatch so the resolver agent can handle line drift explicitly. The agent's rubric now bounds relocation to a single in-file anchor search: found -> re-evaluate there; not found with in-place code described -> `not-addressing`; not found with extraction suggested -> `needs-human`. No repo-wide grep, since the reviewer's surrounding context is gone and silently patching the wrong usage site is worse than asking.
The bot filter assumed AI review bots put all actionable feedback in inline review threads and that their top-level PR comments and review submission bodies were always wrappers. Codex breaks that assumption: it sometimes posts findings (with priority badges and permalinks) as a top-level PR comment with no inline thread counterpart. The blanket drop silently hid those findings (e.g., PR #616 issue comment 4285523213 with P1/P2 items on src/targets/codex.ts and src/commands/cleanup.ts). Replace the blanket bot drop with a content-signal check: keep an AI review bot's body iff it contains a priority badge (`![P1 Badge]` etc.) or a GitHub blob permalink to a specific line. Wrappers carry neither; actionable findings carry both. CI/status bots (codecov) stay dropped unconditionally -- their summaries are never actionable on their own.
Two findings from the Codex review bot's top-level comment: - Flat Codex skill cleanup now gates on SKILL.md frontmatter- description ownership via a new classifyCodexLegacyFlatSkillOwnership helper in utils/legacy-cleanup.ts (pure fingerprint, matching the prompt classifier pattern). A user-authored ~/.codex/skills/ce-plan/ sharing a legacy CE name is preserved; CE-emitted flat skills and genuinely-orphaned pre-manifest entries still get swept. Three-way verdict: ce-owned -> move, foreign -> skip, unknown -> fall through. - cleanup --target codex now derives its default agentsHome from the resolved codexHome via path.dirname(codexHome)/.agents, mirroring install's derivation in cleanupLegacyAgentsSkillSymlinks. Symmetric between install and cleanup when --codex-home is set outside HOME. Explicit --agents-home still wins; default ~/.codex still resolves to ~/.agents.
|
Addressing both findings in #issuecomment-4285523213:
Fixed. Added Chose pure fingerprint (not hybrid manifest+fingerprint) because flat skills predate the namespaced install manifest, so manifest lookups would always miss for exactly the pre-migration case that matters.
Fixed. Cleanup now derives its default Validation: |
Summary
Codex users can now install compound-engineering the native way —
codex plugin marketplace add+ the/pluginsTUI — and get every skill and MCP server registered directly. But Codex's plugin spec does not cover custom agents, and CE's specialized reviewers and resolvers (ce-kieran-*, ce-adversarial-reviewer, ce-pr-comment-resolver, etc.) live as custom agents.So installation is a two-step flow:
~/.codex/agents/. The--to codexdefault is changed to agents-only so the two steps don't double-register skills.This stacks on #609.
Why two steps
Codex has two related but separate extension systems:
plugin marketplace add+/pluginsTUI enable~/.codex/agents/There is no single install command that covers both. The native plugin install does not register custom agents, and CE agents are authored as Claude-format markdown (frontmatter + body), not Codex TOML — so they need converting. The Bun CLI exists to bridge exactly that gap: it reads
plugins/compound-engineering/agents/**/*.md, converts each to a Codex TOML custom agent, and writes the result under~/.codex/agents/compound-engineering/. With--to codexnow defaulting to agents-only, running both steps is additive, not overlapping.Install flow after this PR
Codex's CLI has no
plugin installsubcommand — the TUI is the canonical enable path, and it writes the same[plugins."<name>@<marketplace>"] enabled = trueentry a hypothetical CLI would produce. The README documents this plainly.What's new in the manifests
The repo already shipped dual-format manifests (Claude + Cursor). Codex is a third format following the same pattern:
.agents/plugins/marketplace.json— top-level marketplace (Codex spec has no marketplace-level version field)plugins/compound-engineering/.codex-plugin/plugin.json+plugins/coding-tutor/.codex-plugin/plugin.json— per-plugin manifests declaringskills: "./skills/"and theinterface{}block Codex uses for the TUI listingSchema empirically verified against the
openai-curatedmarketplace cached locally: the samesource: { source: "local", path: "./plugins/<name>" }shape works for remote-fetched marketplaces too.Key technical decisions
--to codexdefault is now agents-only;--include-skillsis opt-in. Native install handles skills, so the Bun converter should only fill the agent gap. The old default would double-register every skill.syncstill pinscodexIncludeSkills: truebecause it's the legacy personal-config path not meant to coexist with native install..claude-plugin/plugin.jsonand.codex-plugin/plugin.jsongets corrected. Version drift is reported only — release-please owns version bumps viaextra-files, and havingsyncReleaseMetadataalso write versions would create drift release-please can't reconcile. Mirrors the Cursor precedent atsrc/release/metadata.ts:214.metadata.version), the Codex marketplace has no version field per its spec. Treated as static content; only per-plugin.codex-plugin/plugin.jsonversions need automated bumping.codexIncludeSkills: trueexplicitly.cleanupCodexandcleanupCodexSharedAgentsneed the full bundle to make correct current-vs-legacy decisions — the agents-only default would make cleanup treat every existing skill as "no longer current" and move it to backup.codexIncludeSkillsis false, the bundle's newexternallyManagedSkillNamesfield records which skills are owned by the native/pluginsinstall.writeCodexBundleunions those intocurrentSkillsbefore runningcleanupLegacyAgentSkillDirs, so re-runninginstall --to codexdoesn't move currently-active skills into legacy-backup. (Addresses a PR review finding.)coding-tutorgets a Codex manifest for marketplace parity. Native install registers its skills; its slash commands still require the Bun converter with--include-skills. README notes the gap.Validator expansion
syncReleaseMetadatanow cross-checks the Codex manifests against Claude as source of truth:.claude-plugin/marketplace.jsonand.agents/plugins/marketplace.jsonnamematch across paired plugin.json filesdescriptionmatch across paired plugin.json fileswrite: trueversionmatch across paired plugin.json filesskillsdeclared and pointing at an existing directoryskillsis missing entirely — addresses a PR review finding)New
errors: string[]field onMetadataSyncResultcarries structural errors;scripts/release/validate.tssurfaces them separately from drift.Contributor docs
plugins/compound-engineering/AGENTS.mdgains parallel Codex rules in Versioning Requirements + Pre-Commit Checklist, and a new "Adding a New Plugin" subsection listing the three-marketplace parity checklist contributors must walk when introducing a new plugin. The validator enforces every rule the doc states.Testing
bun testpasses (821 tests). New coverage:tests/release-metadata.test.ts— new tests covering the Codex cross-checks: happy path, description drift auto-correct, version drift detect-only, missing manifest, name mismatch, missing skills dir, missingskillsfield entirely, plugin-list mismatch in either directiontests/codex-converter.test.ts— new tests for the agents-only default, pluscodexIncludeSkills: truethreaded through existing full-mode teststests/codex-writer.test.ts— new test verifying active skill directories survive an agents-only re-installtests/cli.test.ts— new test forinstall --to codexdefault emitting no skills; existing Codex tests updated for the new defaultEmpirical verification (merge gate per the plan): tested end-to-end locally —
marketplace add→ Bun agent step →/pluginsinstall → Codex restart. Skills register at expected paths, delegating skills find their agents,codex plugin marketplace removecleans up cleanly. Verified the 90+ stale agent-as-skill directories from a pre-#609 install get migrated tolegacy-backup/<timestamp>/correctly on the first install with the new default.Follow-ups captured but not in this PR
Two brainstorm stubs in
docs/brainstorms/2026-04-20-*worth reviewing separately:bunx ... install— cleanup is recurring (every CE release that renames or removes a name) rather than one-time migration, so making it automatic reduces support loadrg/grepsearches