Skip to content

[bug] Windows: codex MCP injection -c value loses its quotes through cmd.exe when node lives under a spaced path #681

Closed

Description

@ranxianglei

来源: #679 分析 Windows 启动截断问题时发现

Context

While fixing #679 (runClient's shell:true truncated spaced client/-e paths), I audited every arg class the launcher passes to clients on Windows. One class cannot be transmitted through cmd.exe faithfully by ANY quoting scheme, and it is broken today already.

What happens

Default bili codex injects the native bili MCP tools inline (src/launcher.ts, buildCodexMcpArgs):

-c mcp_servers.bili.command=<JSON.stringify(process.execPath)>

When node lives under a spaced path (e.g. C:\Program Files\nodejs\node.exe — the stock Windows install location), this single argv token contains double quotes, backslashes AND spaces:

mcp_servers.bili.command="C:\\Program Files\\nodejs\\node.exe"

On Windows codex resolves to an npm .cmd shim, so the spawn goes through cmd.exe (before AND after the #679 fix — shims always need cmd). cmd.exe treats " purely as a quote-state toggle and has NO escape mechanism for a literal quote inside an argument. However the line is built, cmd strips the token's surrounding quotes before the shim sees it, so codex receives:

mcp_servers.bili.command=C:\Program Files\nodejs\node.exe

— an UNQUOTED TOML value containing spaces. That is not a valid TOML string.

Impact (needs Windows verification)

Unverified on real Windows which way codex fails: it either rejects the -c assignment (launch error) or drops/mis-parses the mcp_servers.bili entry (bili MCP server never registers → codex runs WITHOUT the compress tool → in plugin mode compression silently never activates — the same silent failure class as #679's second half). Either way: Windows users with node under a spaced path + default MCP mode are affected.

Note: this is independent of #679's fix — even with perfect per-token quoting, a token containing both quotes and whitespace cannot survive cmd.exe; no quoting scheme exists for it.

Suggested direction

Stop encoding absolute paths into inline -c values. Deliver the MCP server definition via a FILE instead (a temp config file if codex exposes an alternate-config/profile flag, or a temp profile/home dir) — analogous to claude's existing temp --mcp-config file approach (src/launcher.ts, injectMcp branch).

First step: verify actual codex behaviour on Windows (does it reject the -c, or start tool-less?) before choosing the fix.

Activity

  1. ranxianglei commented on Sep 10, 2026

    @ranxianglei
    OwnerAuthor

    🤖 Powered by ework · qwen3.8-27b

    [bot] 🏷 Verified by code analysis — the bug is real, and it's slightly broader than the title frames. Proceeding to implement; PR to follow.

    Triage (what I checked before touching code)

    1. Repro / verification. Confirmed the exact code path:

    2. Layering — broader than spaced paths. It isn't only spaced paths. A valid TOML string requires enclosing quotes, and cmd.exe strips enclosing quotes before the child sees argv. So:

    • Spaced path (C:\Program Files\nodejs) → token splits / quotes stripped → unquoted value with spaces → invalid TOML. (the stock install location — why it's the headline case)
    • Any Windows path (C:\Users\john\AppData\Roaming\npm\..., no space) → the required " quotes are still stripped by cmd → codex receives an unquoted backslashed value → also invalid TOML.

    So the defect class is "any absolute Windows path in an inline -c value," with the spaced stock location being the most common trigger. The root cause is a hard incompatibility: cmd.exe cannot transmit a literal quote character to a child, but every valid TOML string needs one. No quoting scheme fixes it — your framing is correct.

    3. Which -c values are affected. Only buildCodexMcpArgs embeds paths. The other codex -c sources are cmd-safe: buildCodexArgs (URL rewrites — no spaces/quotes) and resolveCodexBudgetArgs (numeric). So the fix is surgical: move the MCP server definition off the inline -c args.

    Fix direction (matches your suggestion)

    Deliver [mcp_servers.bili] via a file, not inline -c. Codex has no --config-file flag; the lever is CODEX_HOME (already relied on by src/plugin-install.ts:362 and src/client-config.ts:564). An ephemeral empty temp home won't work — it would drop the user's auth.json + model/base_url config. So I'll use the persistent <~/.codex>-bili overlay pattern bili already ships for pi/omp/dsh (refreshOverlayHome + writeOverlayFileAtomic): symlink every real-home entry except config.toml (auth/sessions/model settings stay shared), write a merged config.toml (real contents + the [mcp_servers.bili] block with command/args/env.BILI_MCP_PROXY/env.BILI_CONVERSATION_ID, JSON-escaped exactly like plugin-install.ts:368). Set CODEX_HOME=<overlay> on the spawned client. POSIX keeps the working inline -c path unchanged (gated on process.platform === "win32"), so macOS/Linux behavior stays byte-identical.

    If the overlay can't be built (hollow on an unprivileged box), degrade gracefully: skip native MCP injection, don't redirect CODEX_HOME, log a warning — codex then runs in wire mode and compression still activates server-side.

    What remains for real-Windows confirmation

    I'm on Linux, so I can't run codex-windows here. What a Windows box would confirm is which observable failure happens today (reject -c at launch vs. start tool-less) — but that does not change the fix: the input is provably malformed TOML after cmd processing either way, and file-based delivery sidesteps cmd entirely. The PR will include platform-independent unit tests for the overlay/merge logic plus a win32-gated integration test (same pattern as #679's) you can run on a Windows runner.

    Implementing now.

    中文摘要:代码分析确认了该 bug,且范围比标题更大——不只是带空格路径,而是任何 Windows 绝对路径放进内联 -c 值都会被 cmd.exe 剥掉 TOML 必需的引号而变成非法 TOML(根因是 cmd 无法向子进程传递字面引号,而合法 TOML 字符串必须有引号,属硬性不兼容)。仅 buildCodexMcpArgs 受影响(其余 codex -c 是 URL/数字,安全)。修复方向与你的建议一致:改用文件交付 [mcp_servers.bili]——通过持久化 ~/.codex-bili overlay(复用 pi/omp/dsh 的 refreshOverlayHome)合并出含 MCP 块的 config.toml 并设 CODEX_HOME 指向它;POSIX 保持现有内联 -c 不变(按 win32 门控),overlay 失败则优雅降级为 wire 模式。正在实现。

  2. ranxianglei commented on Sep 10, 2026

    @ranxianglei
    OwnerAuthor

    🤖 Powered by ework · qwen3.8-27b

    [bot] 🏷 Triage verdict: bug confirmed by code analysis — and broader than spaced paths. Fix implemented via file delivery (your suggested direction), behind a platform gate so POSIX stays byte-identical.

    What I verified

    • Only buildCodexMcpArgs (src/launcher.ts) embeds an absolute path into an inline -c value. The other codex -c sources are cmd-safe: buildCodexArgs (URL rewrites — no spaces/quotes) and resolveCodexBudgetArgs (numeric). So the fix is surgical to one call site class.
    • The breakage is not limited to spaced paths. Any Windows absolute path in an inline -c value fails, because a valid TOML string requires enclosing quotes and cmd.exe strips them unconditionally (it has no literal-quote escape). The stock C:\Program Files\nodejs\node.exe location is just the most common trigger. Your framing ("cannot be transmitted … by ANY quoting scheme") is exactly right — I confirmed there is no quoting fix; the only escape is to stop putting the path in argv.

    Layering: root cause = hard incompatibility between cmd.exe (can't pass a literal quote to a child) and TOML (every string needs quotes). Symptom = unquoted/misparsed mcp_servers.bili.command → MCP server never registers → plugin-mode compression silently never activates. I fixed the cause (delivery mechanism), not the symptom.

    Fix (per your suggested direction — deliver via a FILE):

    • On win32, [mcp_servers.bili] is written into a persistent <CODEX_HOME>-bili overlay's merged config.toml, and the spawned codex is pointed at it via CODEX_HOME. Reuses the proven pi/omp/dsh overlay machinery (refreshOverlayHome/writeOverlayFileAtomic). Every real-home entry except config.toml is shared, so auth.json, sessions, and model settings survive.
    • POSIX keeps the working inline -c args unchanged (gated on process.platform === "win32"), so macOS/Linux output is byte-identical.
    • If the overlay can't be built (e.g. hollow on a locked-down box), injection degrades gracefully to wire mode — codex still launches and compression stays active server-side — with a logged warning instead of a broken launch.
    • The merged block replaces any pre-existing [mcp_servers.bili] (e.g. from bili plugin install codex) rather than duplicating it, and adds the per-spawn BILI_CONVERSATION_ID the persistent install lacks.

    Pre-flight: typecheck clean · full suite 1240/1241 (the one failure is a pre-existing, environment-dependent resolveClientCommand test — identical on the clean tree, unrelated) · build success. 6 new unit tests lock down the generated config shape, block replacement, sibling sharing, real-home immutability, and platform gating.

    Remaining manual step: real-Windows end-to-end confirmation that codex accepts the CODEX_HOME overlay (registers the bili MCP server) — that's the part unit tests can't cover here.

    PR: #683

    中文摘要:确认了 Windows 下 codex MCP 内联 -c 值经 cmd.exe 丢引号的 bug(且不限于含空格路径,任何绝对路径都会坏),改为在 win32 上通过 <CODEX_HOME>-bili 覆盖层的 config.toml 文件投递 [mcp_servers.bili]、POSIX 保持原样,已提 PR #683,待真机 Windows 验证。

  3. added a commit that references this issue on Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions