Skip to content

fix(cli): escape codex args and stop aborting on exit in launch-codex - #8856

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.49from
sumanxg:fix/cli-launch-codex-windows-spawn
Jul 28, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.49from
sumanxg:fix/cli-launch-codex-windows-spawn

Conversation

@sumanxg

@sumanxg sumanxg commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Problem

launch-codex spawns codex.cmd with shell: true on Windows, so Node joins argv with plain spaces and no escaping (the DEP0190 warning). This is the same defect fixed for launch in #8837, which disclosed it here but left it unfixed.

It is worse for launch-codex than for launch: it breaks every Windows invocation, not only the ones with a multi-word user argument, because buildCodexProviderArgs() injects -c flags whose TOML values are quoted. Captured through a probe .cmd shim that forwards %* exactly like the real npm shim:

["-c","model_provider=omniroute",
 "-c","model_providers.omniroute.name=OmniRoute",
 "-c","model_providers.omniroute.base_url=http://localhost:20128/v1",
 "-c","model_providers.omniroute.env_key=OMNIROUTE_API_KEY",
 "-c","model_providers.omniroute.wire_api=responses",
 "-c","model_providers.omniroute.requires_openai_auth=false",
 "fix","the","bug"]

Three separate corruptions:

  1. cmd.exe strips the TOML quotes, so model_provider="omniroute" reaches codex as model_provider=omniroute and no longer parses as a TOML string.
  2. A multi-word pass-through argument (fix the bug & ship it) is split into separate argv entries.
  3. Everything after an unquoted & is swallowed — cmd.exe reads it as a command separator.

Second defect, also ported from #8837: process.exit(exitCode) in the command action aborts with the libuv !(handle->flags & UV_HANDLE_CLOSING) assertion (async.c:94, exit 0xC0000409) on any non-zero child exit, because it tears the loop down while the inherited stdio handles of the just-exited child are still closing.

Fix

  • Extract the escaping helper from launch.mjs into bin/cli/utils/winShellArgs.mjs and reuse it from both launchers, instead of duplicating a subtle two-layer escape. quoteClaudeArgs keeps its signature and behaviour.
  • launch-codex.mjs: quote the whole codex argv (provider flags + profile + pass-through args) on the win32 shell path via the new quoteCodexArgs. Off Windows there is no shell, so argv is passed through untouched.
  • launch-codex.mjs: process.exit() → process.exitCode, letting the loop drain.

Validation

tests/unit/cli/launch-codex-windows-spawn-args.test.ts, mirroring the launch test added in #8837:

  • golden strings pin the exact win32 encoding, including the double caret-escape the .cmd shim's %* re-parse requires — the cmd.exe round-trip is Windows-only and skips on Linux CI, so without goldens CI would guard nothing;
  • a real cmd.exe round-trip through a probe .cmd asserts the child argv is byte-identical to what the caller passed, using the actual buildCodexProviderArgs() output plus quotes, ampersands, pipes, %PATH%, ^, !, a trailing backslash and an empty argument.

Run on Windows 11 / Node 24.18.0:

node --import tsx/esm --test tests/unit/cli/launch-codex-windows-spawn-args.test.ts \
  tests/unit/cli/launch-codex.test.ts tests/unit/cli/launch-windows-spawn-args.test.ts \
  tests/unit/cli/launch-command.test.ts
# tests 23 | pass 23 | fail 0

`launch-codex` spawns `codex.cmd` with `shell: true` on Windows, so Node joins
argv with plain spaces and no escaping (DEP0190). This mangles every Windows
invocation, not only the ones with a multi-word user argument, because the
injected `-c` provider flags carry quoted TOML values:

  ["-c","model_provider=omniroute", ...,
   "model_providers.omniroute.base_url=http://localhost:20128/v1",
   "fix","the","bug"]

cmd.exe strips the TOML quotes (`model_provider=omniroute` no longer parses as
a TOML string), splits multi-word arguments, and swallows everything after an
unquoted `&`. The same defect was fixed for `launch` in diegosouzapw#8837; this ports it to
`launch-codex`, which that PR disclosed but left unfixed.

- extract the escaping into `bin/cli/utils/winShellArgs.mjs` and reuse it from
  both launchers instead of keeping a private copy in `launch.mjs`
- quote the codex argv (provider flags + profile + pass-through args) on the
  win32 shell path; argv is untouched off Windows, where no shell is involved
- replace `process.exit()` in the command action with `process.exitCode`: on any
  non-zero child exit it aborted with the libuv `!(handle->flags &
  UV_HANDLE_CLOSING)` assertion while the inherited stdio handles were closing

Test: `tests/unit/cli/launch-codex-windows-spawn-args.test.ts` pins the exact
encoding with golden strings (the cmd.exe round-trip is Windows-only and skips
on Linux CI, so without goldens CI would guard nothing) and round-trips the real
provider flags through a probe `.cmd` shim that forwards `%*`.
@sumanxg
sumanxg requested a review from diegosouzapw as a code owner July 28, 2026 14:13
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit 8e0d7e4 into diegosouzapw:release/v3.8.49 Jul 28, 2026
15 of 16 checks passed
@sumanxg
sumanxg deleted the fix/cli-launch-codex-windows-spawn branch July 28, 2026 15:15
diegosouzapw added a commit that referenced this pull request Jul 28, 2026
…e lost credits

Aggregates every pending changelog.d fragment into the [3.8.49] section and
regenerates the contributors table from the reconciled bullets.

Three fixes this surfaced:

- The [3.8.49] section had no `### 📝 Maintenance` heading, so the aggregator's
  findIndex matched the first one in the file — inside [3.8.47] — and would have
  filed 92 maintenance bullets under the wrong release. Added the heading to the
  living section; [3.8.47] stays at its original 234 bullets.

- 46 bullets carried no PR/issue reference. Fragments may keep the number only in
  the filename (`<N>-slug.md`), which the aggregator does not copy into the bullet,
  so the link and the credit were dropped on aggregation. Restored, scoped strictly
  to the [3.8.49] range.

- 9 external contributors lost their attribution that way and are credited again:
  @MisileLab (#8566), @MumuTW (#8619), @epsilonode (#8724), @hppsc1215 (#8835),
  @sumanxg (#8837, #8856), @TitoTFP (#8838), @HouMinXi (#8842, #8845).

Contributors table: 84 → 155 entries, no one removed. 42 i18n mirrors synced.
check:changelog-integrity green — no base bullet lost.
@diegosouzapw diegosouzapw mentioned this pull request Jul 28, 2026
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…diegosouzapw#8856)

* fix(cli): escape codex args and stop aborting on exit in launch-codex

`launch-codex` spawns `codex.cmd` with `shell: true` on Windows, so Node joins
argv with plain spaces and no escaping (DEP0190). This mangles every Windows
invocation, not only the ones with a multi-word user argument, because the
injected `-c` provider flags carry quoted TOML values:

  ["-c","model_provider=omniroute", ...,
   "model_providers.omniroute.base_url=http://localhost:20128/v1",
   "fix","the","bug"]

cmd.exe strips the TOML quotes (`model_provider=omniroute` no longer parses as
a TOML string), splits multi-word arguments, and swallows everything after an
unquoted `&`. The same defect was fixed for `launch` in diegosouzapw#8837; this ports it to
`launch-codex`, which that PR disclosed but left unfixed.

- extract the escaping into `bin/cli/utils/winShellArgs.mjs` and reuse it from
  both launchers instead of keeping a private copy in `launch.mjs`
- quote the codex argv (provider flags + profile + pass-through args) on the
  win32 shell path; argv is untouched off Windows, where no shell is involved
- replace `process.exit()` in the command action with `process.exitCode`: on any
  non-zero child exit it aborted with the libuv `!(handle->flags &
  UV_HANDLE_CLOSING)` assertion while the inherited stdio handles were closing

Test: `tests/unit/cli/launch-codex-windows-spawn-args.test.ts` pins the exact
encoding with golden strings (the cmd.exe round-trip is Windows-only and skips
on Linux CI, so without goldens CI would guard nothing) and round-trips the real
provider flags through a probe `.cmd` shim that forwards `%*`.

* docs(changelog): add fragment for diegosouzapw#8856

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…e lost credits

Aggregates every pending changelog.d fragment into the [3.8.49] section and
regenerates the contributors table from the reconciled bullets.

Three fixes this surfaced:

- The [3.8.49] section had no `### 📝 Maintenance` heading, so the aggregator's
  findIndex matched the first one in the file — inside [3.8.47] — and would have
  filed 92 maintenance bullets under the wrong release. Added the heading to the
  living section; [3.8.47] stays at its original 234 bullets.

- 46 bullets carried no PR/issue reference. Fragments may keep the number only in
  the filename (`<N>-slug.md`), which the aggregator does not copy into the bullet,
  so the link and the credit were dropped on aggregation. Restored, scoped strictly
  to the [3.8.49] range.

- 9 external contributors lost their attribution that way and are credited again:
  @MisileLab (diegosouzapw#8566), @MumuTW (diegosouzapw#8619), @epsilonode (diegosouzapw#8724), @hppsc1215 (diegosouzapw#8835),
  @sumanxg (diegosouzapw#8837, diegosouzapw#8856), @TitoTFP (diegosouzapw#8838), @HouMinXi (diegosouzapw#8842, diegosouzapw#8845).

Contributors table: 84 → 155 entries, no one removed. 42 i18n mirrors synced.
check:changelog-integrity green — no base bullet lost.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants