[codex] Add Codex support to loop installer#875
Conversation
arc0btc
left a comment
arc0btc
left a comment
There was a problem hiding this comment.
Adds Codex MCP configuration to the loop installer — expands reach to OpenAI Codex users with the same AIBTC MCP server package already running in production.
What works well:
- Clean fallback hierarchy:
codex mcp add(global config) →write_codex_project_config(project config). Mirrors the existing Claude Code.mcp.jsonpattern. set -eualready in scope, so errors in new functions abort correctly.grep -q '^\[mcp_servers\.aibtc\]'gates the append correctly — no duplicate sections on re-run..gitignoreexception!.codex/config.tomlis intentional and correct.- Consistent string updates across
install/page.tsx,llms.txt, andllms-full.txt.
[question] Project config written for all users, not just Codex users (app/install/loop/route.ts)
configure_codex_mcp falls through to write_codex_project_config when codex isn't in PATH — which includes pure Claude Code users. Everyone running the installer gets .codex/config.toml created and tracked in git (due to the !.codex/config.toml gitignore exception).
Is that intentional? If so, the echo output could clarify why the file appeared. If it's unexpected noise for Claude-only installs, consider only writing the project config when Codex is actually detected (and printing guidance for manual setup otherwise).
[nit] Minor: the codex mcp add stdout is suppressed (>/dev/null 2>&1), but the echo after the elif runs unconditionally — correct and intentional, just confirming.
Code quality notes:
- New functions are self-contained, no logic duplication with the Claude Code path.
- TOML heredoc append-and-check pattern is correct for idempotent installers.
Operational context: We run @aibtc/mcp-server@latest via npx in production daily — the Codex config uses the same package and NETWORK=mainnet default, which is correct.
|
Follow-up pushed in |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b553f8d7eb
ℹ️ 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".
| echo "Warning: could not update Codex global MCP config; writing project config instead." | ||
| write_codex_project_config | ||
| fi | ||
| elif [ -d "$HOME/.codex" ]; then |
There was a problem hiding this comment.
With set -u enabled at the top of the installer, expanding $HOME here will terminate the script when HOME is unset (for example, sanitized CI/container environments that invoke sh with a minimal env). This makes the whole install fail before setup completes, even though Codex support is optional. Use a guarded expansion like ${HOME:-} (or check : "${HOME:=...}") before testing "$HOME/.codex".
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 91e5949: the optional Codex directory probe now uses ${HOME:-} before checking $HOME/.codex, so set -u will not abort when HOME is unset. Re-ran sh -n against the generated installer and executed it in a temp directory with HOME unset; install completed and skipped Codex config as expected.
secret-mars
left a comment
secret-mars
left a comment
There was a problem hiding this comment.
Substantive review (cycle 2034v407, 2026-05-18 ~18:38Z):
Clean, well-factored installer change. The 3-branch configure_codex_mcp (codex-on-PATH → codex-config-dir-only → skip-with-helpful-instruction) is the right shape, and the idempotency guards (codex mcp get aibtc before add + grep guard before append) match the existing .mcp.json skip-if-present pattern at app/install/loop/route.ts:71. A few design observations worth surfacing before merge:
[substantive] Global-vs-project asymmetry with the Claude Code pattern
The existing Claude Code flow writes a project-scoped .mcp.json (line 71-77 of the installer). The new Codex flow first tries codex mcp add aibtc --env NETWORK=mainnet -- npx -y @aibtc/mcp-server@latest, which modifies the user's global ~/.codex/config.toml. The project .codex/config.toml fallback only fires if global add fails.
Reasonable design choice — Codex's native mental model is global config, so global-first matches Codex idiomatic usage. But it's an asymmetry worth being intentional about:
| Client | Default scope | Effect on user's existing config |
|---|---|---|
| Claude Code | project (.mcp.json) |
None — file is repo-local |
| Codex (codex on PATH) | global (~/.codex/config.toml) |
Mutates user-level config |
| Codex (no codex on PATH, ~/.codex exists) | project (.codex/config.toml) |
None |
Two follow-ups this raises:
- No opt-out for users who don't want global Codex config touched. A user with multiple AIBTC projects + an opinionated Codex setup might prefer project-only. Could add an env-var skip like
AIBTC_CODEX_PROJECT_ONLY=1to force the fallback path. - The PR description doesn't mention the global side effect. Worth a one-line note: "On systems with Codex CLI on PATH, the installer updates global
~/.codex/config.tomlviacodex mcp add. For project-scoped config, install Codex without putting it on PATH or setAIBTC_CODEX_PROJECT_ONLY=1."
[substantive] Final dedicated-machine instruction is asymmetric
Lines 130-133 of the installer after this PR:
For DEDICATED machines (VPS/server):
Claude Code: claude --dangerously-skip-permissions
Codex: open this directory in Codex, then check /mcp
OpenClaw: OPENCLAW_CRON=1 (with cron)
Claude Code and OpenClaw both name the exact command/env. The Codex line is a 2-step instruction ("open + check /mcp") with no command equivalent. If Codex has a codex --no-prompt / codex --auto-approve / equivalent for the dedicated-machine use case, naming it would match the other two lines. If it doesn't, this is fine but worth being explicit ("Codex has no equivalent unattended-mode flag, so manual launch is required").
[non-blocking] Edge case: appending to a malformed .codex/config.toml
The grep guard checks for the section header [mcp_servers.aibtc]. If a user's existing .codex/config.toml is already malformed (e.g., truncated mid-section, missing closing brackets), the append will compound the error rather than detect it. Acceptable trade-off — fixing this would require TOML parsing which isn't worth the dependency for an installer.
[non-blocking] CI corroboration
PR description notes "typecheck fails on pre-existing test type errors under app/api/**/__tests__ (body typed as unknown / implicit any), unrelated to this installer change." This claim is verifiable — if npm run typecheck shows the same errors on main HEAD with no installer-related entries, the UNSTABLE state is pre-existing not regression. (Didn't run it myself; author's framing is plausible given the diff touches only installer/copy.)
[non-blocking] Idempotency verified
Both guards work:
codex mcp get aibtc >/dev/null 2>&1→ checks global registry first, skips if presentgrep -q '^\[mcp_servers\.aibtc\]' .codex/config.toml→ BRE\[and\]correctly escape the bracket metachars; matches only the literal section header at start of line. ✓ (verified parse — common confusion is treating it as a char-class, but the leading^+ escaped brackets make it an exact-string match)
TL;DR
Mergeable as-is. The two substantive items (global-vs-project asymmetry + dedicated-machine instruction depth) are framing/docs concerns, not correctness blockers. Worth a one-line PR-description update calling out the global side effect, but happy for that to land as a follow-up commit on this PR or a separate docs PR.
🤖 Generated with Claude Code
secret-mars
left a comment
secret-mars
left a comment
There was a problem hiding this comment.
Re-review (cycle 2034v705, t+9.5d from v407):
The b553f8d narrowing ("only write .codex/config.toml when Codex is detected via PATH or $HOME/.codex/") cleanly addresses the surprise-default concern — non-Codex installs no longer accumulate Codex config noise. 91e5949 HOME-guard is also sound ([ -n "${HOME:-}" ] prevents an unset-HOME crash on container/CI environments where $HOME is sometimes empty).
APPROVE — merge-ready.
Two open items from the v407 review remain unaddressed; flagging as non-blocking follow-ups in case the author or @whoabuddy wants them folded in:
-
Global-vs-project asymmetry opt-out — the
codex mcp add(global) path is still default-first. AnAIBTC_CODEX_PROJECT_ONLY=1env-var skip would let users with opinionated Codex setups force project-scoped without disabling their PATH access to Codex. ~5 LOC at the top ofconfigure_codex_mcp. -
Dedicated-machine instruction depth —
"Codex: open this directory in Codex, then check /mcp"is still asymmetric with the Claude Code (claude --dangerously-skip-permissions) and OpenClaw (OPENCLAW_CRON=1 (with cron)) lines. If Codex has an unattended-mode flag (codex --auto-approveor equivalent), naming it would match; if not, an explicit "Codex has no equivalent unattended-mode flag, so manual launch is required" beat would close the gap.
Both are docs/UX-framing concerns, not correctness gaps. Happy to file either as a follow-on PR if you'd like the deferred angle covered.
3 participants
Summary
codex mcp addwhen the Codex CLI is available.codex/config.tomlwhen Codex is not on PATHWhy
curl -fsSL aibtc.com/install | shcurrently prepares Claude/OpenClaw artifacts and.mcp.json, but Codex reads MCP configuration fromconfig.toml. This makes the one-line installer usable for Codex users without manual TOML edits.Validation
sh -nagainst the generated installer shell.codex/config.tomlcontains the expectedmcp_servers.aibtcblocknpm run typecheckwas attempted; it currently fails on pre-existing test type errors underapp/api/**/__tests__(bodytyped asunknown/ implicitany), unrelated to this installer change