Add Codex plugin marketplace support#407
Conversation
hbrodin
left a comment
There was a problem hiding this comment.
Reviewed for correctness, standards adherence, and user-facing behavior. Builds clean (clippy -D warnings), 1003 unit tests pass, and I adversarially re-checked my initial concerns. Two of them collapsed on verification: the [plugins.<name>] enabled config shape is real (not fabricated) and extract_codex_plugin_state clones the whole plugins/marketplaces subtree, so preservation is shape-agnostic; and the missing-CHANGELOG concern was wrong (this repo batches CHANGELOG entries at release-prep, not per-PR). What remains are the three non-blocking nits below. The main thing still worth doing before merge is the manual stop/start plugin-survival check the PR description already lists, since the integration path is untested against a real Codex CLI.
| }, | ||
| Err(e) => { | ||
| tracing::warn!( | ||
| "Could not read guest ~/.codex/config.toml to preserve installed Codex \ |
There was a problem hiding this comment.
Non-blocking consistency note: plugin/marketplace install is BootMode::FirstBoot-only, so on a restart, if this read (or the parse below) fails while a config.toml rewrite is otherwise triggered, the guest's installed [marketplaces.*]/[plugins.*] are dropped with just this warning and no reinstall to recover them. This matches the accepted merge_managed_claude_settings tradeoff, so it's consistent — but note the realistic Err here is a transient SSH read of an otherwise-good file, not the corrupt file that path deliberately resets. The Claude path carries a comment addressing exactly this; a one-line note here pointing at the FirstBoot-only recovery would match it.
| [marketplaces.mine]\n\ | ||
| source = \"trailofbits/codex-plugins\"\n\ | ||
| \n\ | ||
| [plugins.my-lsp]\n\ |
There was a problem hiding this comment.
Nit: a real Codex install keys this table by plugin@marketplace (e.g. [plugins."my-lsp@codex-plugins"], matching the plugins = ["my-lsp@codex-plugins"] field), not the bare [plugins.my-lsp]. The extractor clones the whole plugins subtree so behavior is unaffected, but the quoted key would make the fixture reflect actual guest output.
|
|
||
| ### Plugin marketplaces | ||
|
|
||
| `marketplaces` and `plugins` declare Codex [plugin marketplaces](https://developers.openai.com/codex/plugins) and the plugins to install from them, mirroring the same fields under `[claude]`: |
There was a problem hiding this comment.
Nit: this URL 308-redirects cross-host to learn.chatgpt.com/docs/plugins. It works today, but linking the canonical host directly avoids future link rot.
|
I've tested the plugins in Codex under a Lima machine, but haven't tested a Firecracker VM |
|
running test on linux |
Integration test results — Linux / FirecrackerRan the full suite on a Firecracker host (the "not yet run" item from the PR description).
ResultThe 6 skips are all expected (Firecracker one-time rsync mounts, macOS-only AppleDouble check, The 3 failures — stale test assertions, not a product bugAll three are in the pre-existing The I ruled out the scp tilde-expansion gotcha (documented in Suggested fix (test-only)Update the marketplace phase in
With those two edits the phase should pass; no product-code change is needed. Happy to push this to the branch if you'd like. |
Re-run after 2ac28d3 — greenRe-ran the full suite on the same Firecracker host against the updated branch (
The 3 previously-failing assertions in the 6 skips unchanged and all expected (Firecracker one-time rsync mounts, macOS-only AppleDouble check, |
|
Thanks @frabert ! I've rerun it on firecracker and it seems to be working. |
Mirror the existing Claude plugin/marketplace mechanism for Codex:
`[codex] marketplaces` / `plugins` are baked into the golden image on
the Lima backend and delta-installed on first boot via `codex plugin
marketplace add` / `codex plugin add` (Firecracker bakes nothing and
installs the full set on first boot, as Claude already does there).
The Codex CLI stores marketplace registrations (`[marketplaces.*]`) and
per-plugin enabled state (`[plugins.*]`) in `~/.codex/config.toml` — the
same file coop rewrites on every boot. coop now reads the guest's config
back and preserves those tables across the rewrite (dropping any that
came from the host's own config.toml so host state does not leak in), so
installed plugins and manual `/plugins` toggles survive stop/start. The
guest read is gated on whether a rewrite will actually happen, and warns
rather than silently dropping state when the guest config cannot be read
or parsed.
Local marketplace directories are copied into a per-tool guest subdir
(`~/.coop/marketplaces/{claude,codex}/…`) so same-basename marketplaces
from the two agents do not collide.
Profiles stay Claude-only; Codex plugins are config-level `[codex]`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- read_codex_plugin_state: note that the realistic failure is a transient SSH read (not the corrupt-file reset the Claude path does), and that because install is FirstBoot-only a restart won't reinstall dropped tables — recovery requires recreating the instance. - Tests: key the preservation fixtures by real guest output — `[marketplaces.<name>]` and the quoted `[plugins."plugin@marketplace"]` — instead of the bare `[plugins.my-lsp]` placeholder. - docs/codex-integration.md: link the canonical plugins doc host (learn.chatgpt.com) directly instead of the developers.openai.com URL that 308-redirects to it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The collision fix moved local marketplaces into a per-tool guest subdir
(`~/.coop/marketplaces/{claude,codex}/…`), but the `local marketplace`
phase of tests/integration.sh still asserted the old flat layout,
failing 3 assertions on the Firecracker/Linux run. This is a `[claude]`
marketplace, so update the copy-verification base path and the
Firecracker `marketplace add` log grep to include the `claude/` subdir.
Test-only; no product-code change. Reported by @hbrodin's Firecracker
integration run on this branch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2ac28d3 to
5c39069
Compare
What
Adds first-class support for Codex plugins and marketplaces, mirroring the existing Claude Code mechanism.
[codex] marketplaces/[codex] pluginsconfig fields (with~expansion and local-path validation), documented inconfig.example.toml,docs/codex-integration.md, anddocs/configuration.md.TemplateConfigascodex_marketplaces/codex_plugins, staleness-checked inneeds_rebuild) and delta-installed on first boot viacodex plugin marketplace add/codex plugin add. On Firecracker nothing is baked, so the full set installs on first boot — identical to how Claude already behaves there.[codex].Why the config.toml handling is the tricky part
The Codex CLI stores marketplace registrations (
[marketplaces.*]) and per-plugin enabled state ([plugins.*]) in~/.codex/config.toml— the same file coop rewrites from the host on every boot. A naive port would wipe those tables on every restart (the bug class already fixed for Claude'ssettings.json).So coop now reads the guest's
config.tomlback and preserves its[marketplaces.*]/[plugins.*]tables across the rewrite, while dropping any that came from the host's ownconfig.toml(host state must not leak into the guest). Installed plugins — and manual/pluginsenable/disable toggles — survive stop/start. The guest read is gated on whether a rewrite will actually happen (no wasted SSH round-trip otherwise) and warns rather than silently dropping state when the guest config can't be read or parsed, mirroringmerge_managed_claude_settings.Local marketplace directories are copied into a per-tool guest subdir (
~/.coop/marketplaces/{claude,codex}/…) so same-basename marketplaces from the two agents don't overwrite each other.Reviewer notes
~/.codex/config.toml(rather than in coop's[codex]) are now stripped from the guest, where previously the host config was copied verbatim. This is intentional (declarative[codex]is the source of truth; don't leak host state), but it changes existing behavior.backend.rs/lima.rs, which are whole-module mutation-excluded, so no.cargo/mutants.tomlchange was needed. The pure helpers that landed in swept modules (collect_codex_baked_lists,check_local_marketplaces) are unit-tested, andcompute_codex_plugin_delta's field wiring has a direct test despitebackend.rsbeing excluded.codex plugin ...CLI shape differs from Claude's (addnotinstall; no--scope/-s user), so it needs its own installer functions — the shared local-dir copy logic is factored intostage_marketplace_source.Testing
cargo fmt/cargo clippy --all-targetsclean; 996 unit tests pass (new tests cover config parse/validate/tilde-expansion, config.toml preservation, host-state dropping,extract_codex_plugin_stateround-trip + error cases,plugin_delta, the baked-list collector, andcompute_codex_plugin_deltafield wiring).cargo mutants --in-diffon the changed mutation-swept logic: 16/16 caught, 0 missed.[codex] marketplaces/plugins,coop up, verifycodex plugin listin-guest, thencoop stop+coop startand confirm the plugin survives (exercises the preservation path).🤖 Generated with Claude Code