fix(mcp): upgrade Codex MCP config to include mcp arg

[reysilvaa]

Fixes #1595: in lean-ctx >= 3.9.20, launching the binary without arguments in environments where standard input/output behaves like an interactive terminal or PTY (such as Devin, containerized agents, or subshells) triggers the interactive quickstart rather than the stdio MCP server. Passing explicit mcp (lean-ctx mcp) bypasses this check and reliably starts the stdio MCP server regardless of TTY allocation.

However, Codex configuration (~/.codex/config.toml) historically wrote args = []. When users upgrade from 3.9.18 to 3.9.20, lean-ctx doctor does not flag args = [] or missing args as drift, and running lean-ctx doctor --fix, lean-ctx upgrade, or lean-ctx setup maintained or rewrote args = [].

What changed

• rust/src/core/editor_registry/writers/install/codex.rs:
  • New Codex configuration blocks now default to args = ["mcp"].
  • upsert_codex_toml detects legacy empty args = [] (or args without "mcp") and updates them to args = ["mcp"].
  • Preserves existing custom args containing "mcp" idempotently.
• rust/src/hooks/agents/codex.rs:
  • ensure_codex_mcp_server now upgrades missing or empty args to ["mcp"].
• rust/src/doctor/integrations/codex.rs:
  • check_codex_toml now verifies both command and that args contains "mcp".
  • Flags missing or empty args as drift (~/.codex/config.toml) so lean-ctx doctor surfaces the issue and lean-ctx doctor --fix (and setup --fix) automatically repairs it.
• rust/src/core/editor_registry/writers/tests.rs:
  • Updated existing upsert_codex_toml assertions to expect args = ["mcp"].
  • Added upsert_codex_toml_upgrades_legacy_empty_args_to_mcp testing input with args = [] and verifying upgrade to args = ["mcp"].
• rust/src/hooks/agents/codex.rs:
  • Added ensure_mcp_server_upgrades_empty_args_to_mcp regression test.
• rust/src/doctor/integrations/tests.rs:
  • Added unit tests for check_codex_toml covering missing files, legacy empty args drift, missing args drift, and valid configuration.

Verification Plan

• cargo fmt --check: passed (clean)
• Regression tests added for upsert_codex_toml, ensure_codex_mcp_server, and check_codex_toml
• CI checks running on rey-workbench fork for all platforms

Contributor License Agreement

Signed via CLA bot in PR #1463.

[yvgude]

Thanks for this — the writer side is the right shape, and the new upsert_codex_toml_upgrades_legacy_empty_args_to_mcp test covers the case the report describes. Two things before it can go in, plus one correction to the premise.

1. CI is red on a test the change missed.

rust/src/core/editor_registry/writers/tests.rs:173 (codex_toml_upserts_existing_section) still asserts the old output:

thread 'core::editor_registry::writers::tests::codex_toml_upserts_existing_section' panicked at
  src/core/editor_registry/writers/tests.rs:173:5:
assertion failed: content.contains("args = []")

That's what takes down Test (ubuntu-latest), Test (windows-latest) and Coverage. The cargo-deny failure is unrelated to you — chacha20 0.10.1 was yanked upstream and it fails on every open PR; a lockfile bump is on its way to main.

2. args = [] is not drift, and the doctor change makes it one.

check_codex_toml now requires the mcp argument to be present, so every user who already has a working args = [] gets a red Codex check and a setup --fix that rewrites a config which was doing its job. That is the same failure mode as #1596 (reporting an intentional, functional state as drift), and it would land on far more users than the ones this PR helps.

Bare invocation is still a supported way to start the stdio server — rust/src/cli/dispatch/server.rs:306:

matches!(first_arg, None | Some("mcp"))

So the asymmetry I'd suggest is: writers emit args = ["mcp"] (explicit is better, and it matches what the Copilot writer already does), while the doctor accepts both [] and ["mcp"] and only flags an args array that contains something else. Then new installs are explicit, existing installs stay green, and nobody's file is rewritten to a semantically identical one.

3. The premise of #1595 doesn't reproduce here.

Driving a bare 3.9.20 binary over stdio with no subcommand answers initialize normally:

$ printf '%s\n' '{"jsonrpc":"2.0","id":1,"method":"initialize",...}' | lean-ctx
{"jsonrpc":"2.0","id":1,"result":{"protocolVersion":"2024-11-05",...,"serverInfo":{"name":"lean-ctx","version":"3.9.20"}}}

So 3.9.20 does not require the mcp argument, and whatever broke for the reporter on the 3.9.18 → 3.9.20 upgrade is something else. I'd rather not close #1595 with this PR — I'll keep it open and ask for the Codex-side stderr. This PR is still worth having on its own merits as a consistency/explicitness fix; it just isn't the fix for that report.

Happy to merge once (1) is green and (2) is loosened. If you'd rather not carry (2), say so and I'll take it in a follow-up.

[yvgude]

Taken over in #1616 with your commit and authorship intact — and with an apology, because the review I left you was wrong on its central point.

I told you bare invocation still serves MCP and that your change was consistency-only. It isn't. My probe piped stdin; the reporter's environment allocates a pty, and the TTY branch in cli/dispatch/mod.rs then prints the quickstart and exits. Against a shipped 3.10.1 binary:

stdin = pipe          →  server starts     ✓
stdin = pty, bare     →  quickstart, exit  ✗
stdin = pty + "mcp"   →  server starts     ✓

So args = ["mcp"] was a real workaround for a real defect, exactly as you and the reporter described. Your writer changes land unchanged in #1616.

I tried to push the follow-up to rey-workbench:fix/1595-codex-mcp-args directly, but GitHub refused (403) despite "allow edits from maintainers", so the branch had to move into this repo. Your commit 55ea087 is the first of the three and carries your name.

Two things I changed, both from the review:

1. codex_toml_upserts_existing_section still asserted args = [] — that was the red CI. Fixed, plus a companion test pinning that args = ["mcp", "--verbose"] survives verbatim rather than being flattened.

2. check_codex_toml no longer reports args = [] as drift. It still starts the server, so flagging it would turn every existing Codex install red and have --fix rewrite a working file into an equivalent one. Only an args list that replaces the entry point (["dashboard"]) is drift now.

And #1616 fixes the root cause: the quickstart moves to stderr and the server starts regardless of TTY, with a pty integration test that fails against the old behaviour.

Thank you for the patch and for being right about it. Closing this in favour of #1616 — not because the work was wrong, but because I could not push into your fork.