From 03fd6eaef01013177b707b614afc1a4cc84f74cd Mon Sep 17 00:00:00 2001
From: bluzername <bluzer@gmail.com>
Date: Thu, 10 Sep 2026 07:28:18 +0300
Subject: [PATCH] fix(cli): keep TOML digit separators when coercing
 --codex-config values

TOML let a number use `_` between digits, like
`model_max_output_tokens=100_000`, so it is nicer to read than
`100000`. But coerce() in backends.ts just call Number() on the raw
text, and Number("100_000") is NaN in JS - it not understand that
syntax at all. So the value fall through to the string branch and
Codex get the literal text "100_000" where its config want a number.
This is exactly the same class of bug the function already exist for
(a string reaching Codex where a scalar was expect), just a different
syntax that trip it up.

Fix strip an underscore only when it sit between two digits before
calling Number() on it, using a regex lookaround. A wrongly placed
underscore (leading, trailing, or doubled, like "_100", "100_",
"1__000") is left alone and still fail the numeric check same as
before, so it keep falling back to a string like real TOML would treat
it as invalid too.

Test: added cases in backends.test.ts for an integer, a float, a
negative number, all with separators, plus the three invalid-placement
cases that must stay strings. Ran the new tests against the old
coerce() first to confirm they fail (they did, 3 failing), then again
with the fix to confirm all pass. Full suite: pnpm typecheck and pnpm
turbo run test --filter=@airshiplabs/cli both green, 182 tests passed
(176 before, +6 new).
---
 apps/cli/src/lib/backends.test.ts | 25 +++++++++++++++++++++++++
 apps/cli/src/lib/backends.ts      | 15 +++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/apps/cli/src/lib/backends.test.ts b/apps/cli/src/lib/backends.test.ts
index cdffa21..9ba1273 100644
--- a/apps/cli/src/lib/backends.test.ts
+++ b/apps/cli/src/lib/backends.test.ts
@@ -26,6 +26,31 @@ describe("coerce", () => {
     expect(coerce('"true"')).toBe("true");
     expect(coerce('"42"')).toBe("42");
   });
+
+  // The bug this is for: TOML let a number use `_` between digits, so
+  // `model_max_output_tokens=100_000` is valid TOML and much easier to read
+  // than `100000`. Before this, coerce() did not know that syntax and the
+  // value reach Codex as the literal string "100_000" instead of the number
+  // 100000.
+  it("reads a TOML digit separator in an integer", () => {
+    expect(coerce("100_000")).toBe(100_000);
+    expect(coerce("1_000_000")).toBe(1_000_000);
+  });
+
+  it("reads a TOML digit separator in a float", () => {
+    expect(coerce("1_234.5")).toBe(1234.5);
+  });
+
+  it("keeps a negative number with a digit separator", () => {
+    expect(coerce("-1_000")).toBe(-1000);
+  });
+
+  it.each(["_100", "100_", "1__000"])(
+    "does not touch an underscore that is not a valid separator: %s",
+    (bad) => {
+      expect(coerce(bad)).toBe(bad);
+    }
+  );
 });
 
 describe("parseCodexConfig", () => {
diff --git a/apps/cli/src/lib/backends.ts b/apps/cli/src/lib/backends.ts
index 5865801..590b79e 100644
--- a/apps/cli/src/lib/backends.ts
+++ b/apps/cli/src/lib/backends.ts
@@ -11,6 +11,16 @@ import { readFileSync } from "node:fs";
 import type { CodexConfigValue } from "@airship/server";
 import { CliError, EXIT } from "./errors";
 
+// TOML lets a number use `_` between digits as a separator, so a config value
+// like `model_max_output_tokens=100_000` is valid TOML and reads a lot better
+// than `100000`. `Number()` does not know this syntax, so left alone it falls
+// through to the string branch below and Codex would get the literal text
+// "100_000" where it wants an integer. The lookaround only strips an
+// underscore that sits between two digits, so a wrongly placed one (`_100`,
+// `100_`, `1__000`) is left in place and still fails the numeric check, same
+// as it would in real TOML.
+const DIGIT_SEPARATOR = /(?<=[0-9])_(?=[0-9])/g;
+
 /**
  * Interpret a flag value as the TOML scalar it looks like.
  *
@@ -27,8 +37,9 @@ export function coerce(raw: string): CodexConfigValue {
   if (raw === "false") {
     return false;
   }
-  if (raw !== "" && Number.isFinite(Number(raw))) {
-    return Number(raw);
+  const withoutSeparators = raw.replace(DIGIT_SEPARATOR, "");
+  if (raw !== "" && Number.isFinite(Number(withoutSeparators))) {
+    return Number(withoutSeparators);
   }
   // An explicitly quoted value keeps its quotes stripped but stays a string.
   if (raw.length > 1 && raw.startsWith('"') && raw.endsWith('"')) {