Skip to content

fix(cli): escape claude args and stop aborting on exit in omniroute launch - #8837

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

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

Conversation

@sumanxg

@sumanxg sumanxg commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Two Windows bugs in omniroute launch, both in bin/cli/commands/launch.mjs. Found while launching Claude Code through a local OmniRoute on Windows 11 / Node 24.18.0.

1. Pass-through arguments are truncated at the first space

spawn(..., { shell: true }) (added in #8283 to resolve the claude.cmd shim) joins argv with plain spaces and no escaping — this is exactly what Node's DEP0190 warning printed on every launch is about. Any argument containing a space is split by cmd.exe.

$ omniroute launch --profile auto-best-coding -p "In one short line: say BANANA"
# claude receives: -p In        (plus "one", "short", ... as stray positionals)
# model replies: "It looks like your message got cut off — you typed 'In'..."

Fixed by escaping each argument for cmd.exe: CRT argv rules first (double the backslashes preceding a quote, escape embedded quotes, wrap in quotes), then cmd metacharacters caret-escaped twice. The second pass is required because claude.cmd is an npm shim that re-parses %* on the way to node — with a single pass, arguments still truncate at the first & or |. (Same rule as cross-spawn's doubleEscapeMetaChars.)

After:

$ omniroute launch --profile auto-best-coding -p "In one short line: say BANANA & MANGO"
BANANA & MANGO

2. Any non-zero exit aborts the process with a libuv assertion

The command action called process.exit(exitCode), tearing the loop down while the exited child's inherited stdio handles were still closing:

$ omniroute launch --profile auto-best-coding     # with claude not on PATH
The 'claude' CLI was not found in PATH.
Assertion failed: !(handle->flags & UV_HANDLE_CLOSING), file src\win\async.c, line 94
# exit code 0xC0000409 instead of claude's exit code

Fixed by setting process.exitCode and letting the loop drain. Verified a child exiting 3 now propagates as 3, and the process still terminates promptly (~1.9s, no hang).

This one is not limited to the ENOENT path — it fired on any non-zero exit from claude.

Changes

  • bin/cli/commands/launch.mjs — adds exported resolveClaudeSpawn() (extracts the existing inline platform choice, mirroring the resolveCodexSpawn() precedent) and quoteClaudeArgs(); process.exit() → process.exitCode.
  • tests/unit/cli/launch-windows-spawn-args.test.ts — new, 7 tests.

No new dependencies; the escaping is a few lines rather than pulling in cross-spawn.

Tests

Test files added/changed: tests/unit/cli/launch-windows-spawn-args.test.ts

$ node --import tsx/esm --test tests/unit/cli/launch-windows-spawn-args.test.ts
# tests 7 / pass 7 / fail 0

# no regressions in the neighbouring CLI suites
$ node --import tsx/esm --test tests/unit/cli/launch-command.test.ts \
    tests/unit/cli/launch-codex.test.ts \
    tests/unit/launch-codex-windows-spawn-6312.test.ts
# tests 11 / pass 11 / fail 0

$ npm run lint
# 1 problem (1 error, 0 warnings) — pre-existing, unrelated to this PR:
#   src/app/(dashboard)/dashboard/settings/components/ProviderAccountRoutingCard.tsx
#   87:5 react-hooks/exhaustive-deps — useCallback missing dependency 'load'
# That file is byte-identical to release/v3.8.49 on this branch
# (`git diff origin/release/v3.8.49 --name-only` lists only the two files above).
# Left untouched so this PR stays scoped — flagging it in case it is news.

The test written red-first against both bugs, then green after the fix. Coverage of the encoding is split deliberately so CI still guards it:

  • Golden-string assertions pin the exact escaped output for 8 inputs (flags, spaces, &, embedded quote, trailing backslash, %PATH%, empty string). Pure function, so these run on Linux CI.
  • A Windows-only round-trip test spawns a real npm-style .cmd shim (@echo off + node "%~dp0argv.mjs" %*) through the same shell: true path and asserts argv arrives byte-identical. Skips off-Windows with a stated reason.

I verified the golden assertions actually bite: reducing the double caret-escape back to a single pass turns them red, which is the case Linux CI would otherwise miss entirely.

Notes for reviewers

  • bin/cli/commands/launch-codex.mjs carries the identical defect — same shell: true concatenation, same process.exit(). It is currently masked because its generated -c args contain no spaces, but any user pass-through arg with a space breaks the same way. Left unchanged here to keep this PR scoped; happy to port both fixes in a follow-up if you'd like them together.
  • The double caret-escape is correct only while the Windows target is a .cmd shim. resolveClaudeSpawn() guarantees that today, but the two are coupled by convention rather than structure — worth a comment or an assertion if the spawn target ever becomes configurable.
  • DEP0190 still prints on every Windows launch. It is now inaccurate (the args are escaped), but silencing it means restructuring the spawn, so I left it alone.

…aunch

Windows launches go through spawn(..., { shell: true }), which joins argv
with plain spaces and no escaping (Node DEP0190). Any argument containing a
space was split, so `omniroute launch -p "two words"` reached claude as
`-p two` plus stray positionals, and the prompt was silently truncated.

Escape each argument for cmd.exe instead: CRT argv rules first (double the
backslashes preceding a quote, escape embedded quotes, wrap in quotes), then
cmd metacharacters caret-escaped twice. The second pass is required because
claude.cmd is an npm shim that re-parses %* on the way to node; with a single
pass arguments still truncated at the first `&` or `|`.

The command action also called process.exit() on any non-zero exit. That tore
the loop down while the exited child's inherited stdio handles were still
closing and aborted the process with a libuv assertion
(!(handle->flags & UV_HANDLE_CLOSING), src/win/async.c:94, exit 0xC0000409)
instead of returning claude's exit code. Set process.exitCode and let the
loop drain.

Extracts resolveClaudeSpawn() alongside the existing resolveCodexSpawn()
precedent so both the platform choice and the escaping are unit-testable.

Tests: tests/unit/cli/launch-windows-spawn-args.test.ts (new, 7 tests).
Four pure-function tests plus golden strings pin the exact encoding on every
platform; a Windows-only test round-trips argv through a real npm-style .cmd
shim and asserts embedded quotes, `&`, `|`, `%PATH%`, `^`, `!`, a trailing
backslash and an empty string all arrive byte-identical.

Note: bin/cli/commands/launch-codex.mjs carries the identical defect (same
shell:true concatenation, same process.exit) and is left unchanged here.
@sumanxg
sumanxg requested a review from diegosouzapw as a code owner July 28, 2026 08:25
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit 2675b65 into diegosouzapw:release/v3.8.49 Jul 28, 2026
3 checks passed
@sumanxg
sumanxg deleted the fix/cli-launch-windows-spawn branch July 28, 2026 14:04
diegosouzapw added a commit that referenced this pull request Jul 28, 2026
…#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 #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 #8856

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

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
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
…aunch (diegosouzapw#8837)

* fix(cli): escape claude args and stop aborting on exit in omniroute launch

Windows launches go through spawn(..., { shell: true }), which joins argv
with plain spaces and no escaping (Node DEP0190). Any argument containing a
space was split, so `omniroute launch -p "two words"` reached claude as
`-p two` plus stray positionals, and the prompt was silently truncated.

Escape each argument for cmd.exe instead: CRT argv rules first (double the
backslashes preceding a quote, escape embedded quotes, wrap in quotes), then
cmd metacharacters caret-escaped twice. The second pass is required because
claude.cmd is an npm shim that re-parses %* on the way to node; with a single
pass arguments still truncated at the first `&` or `|`.

The command action also called process.exit() on any non-zero exit. That tore
the loop down while the exited child's inherited stdio handles were still
closing and aborted the process with a libuv assertion
(!(handle->flags & UV_HANDLE_CLOSING), src/win/async.c:94, exit 0xC0000409)
instead of returning claude's exit code. Set process.exitCode and let the
loop drain.

Extracts resolveClaudeSpawn() alongside the existing resolveCodexSpawn()
precedent so both the platform choice and the escaping are unit-testable.

Tests: tests/unit/cli/launch-windows-spawn-args.test.ts (new, 7 tests).
Four pure-function tests plus golden strings pin the exact encoding on every
platform; a Windows-only test round-trips argv through a real npm-style .cmd
shim and asserts embedded quotes, `&`, `|`, `%PATH%`, `^`, `!`, a trailing
backslash and an empty string all arrive byte-identical.

Note: bin/cli/commands/launch-codex.mjs carries the identical defect (same
shell:true concatenation, same process.exit) and is left unchanged here.

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

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
…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