Skip to content

Commit 3bf5df9

Browse files
committed
fix: codex login on Windows hit nested quoting hell
Clicking 'codex 认证' on Chinese Windows produced '文件名、目录名或卷 标语法不正确' before codex ever started. The launcher was spawning: cmd.exe /c start cmd.exe /k <command-with-quoted-path-and-args> which forced the argv through four parsers in a row - Node child_process argv quoting, outer cmd.exe /c, start, and inner cmd.exe /k - each with different quote escape rules. Node emits CRT-style '"' for embedded quotes; cmd.exe does not understand that escape and chopped 'cd /d "C:\...\"' into a literal backslash plus a broken path. 'cd /d' then rejected the path. Reduce parsing to a single layer by writing the command to a temp .bat file (ASCII + CRLF, at a fixed path in $TEMP so successive logins do not accumulate files) and launching it with 'cmd /c start "" <batPath>'. The quotes inside the bat are now interpreted once, by the bat's own cmd.exe, exactly as typed. Also add a unit test that asserts the generated bat content is ASCII-only with CRLF line endings and contains the right cd /d and codex invocation - future regressions fail at npm test time instead of on a real Windows box.
1 parent 43869e2 commit 3bf5df9

2 files changed

Lines changed: 69 additions & 4 deletions

File tree

core/agent/src/codex-login.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import { spawn } from "node:child_process";
2+
import { writeFile } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
25
import type { SpawnProcess, SpawnedProcessLike } from "./codex.js";
36

47
export interface CodexLoginStatus {
@@ -134,12 +137,26 @@ async function launchCodexLoginOnWindows(input: {
134137
codexBin: string;
135138
workspaceRoot: string;
136139
}): Promise<void> {
137-
const command = buildCodexLoginCommandWindows(input);
140+
// Why a .bat file instead of 'cmd /c start cmd /k "..."':
141+
//
142+
// The nested form forced the command line through four parsers in a
143+
// row (Node child_process argv quoting -> outer cmd.exe /c -> start ->
144+
// inner cmd.exe /k). Each layer uses different quote escaping rules
145+
// and cmd.exe in particular chops '\"' into a literal backslash plus
146+
// a quote, which mangled the 'cd /d "C:\...\"' prefix and produced
147+
// '文件名、目录名或卷标语法不正确' on Chinese Windows before codex
148+
// ever started. Writing the command to a .bat file reduces parsing
149+
// to a single layer inside the bat's own cmd.exe, where the quotes
150+
// are interpreted exactly as typed.
151+
const batPath = buildCodexLoginBatPath();
152+
const batContent = buildCodexLoginBatContent(input);
153+
await writeFile(batPath, batContent, { encoding: "ascii" });
138154

139155
await new Promise<void>((resolve, reject) => {
140-
const child = spawn("cmd.exe", ["/c", "start", "cmd.exe", "/k", command], {
156+
const child = spawn("cmd.exe", ["/c", "start", "", batPath], {
141157
cwd: input.workspaceRoot,
142-
stdio: ["ignore", "ignore", "pipe"]
158+
stdio: ["ignore", "ignore", "pipe"],
159+
windowsHide: false
143160
});
144161

145162
let stderr = "";
@@ -169,6 +186,28 @@ async function launchCodexLoginOnWindows(input: {
169186
});
170187
}
171188

189+
export function buildCodexLoginBatPath(): string {
190+
return join(tmpdir(), "screen-pilot-codex-login.bat");
191+
}
192+
193+
export function buildCodexLoginBatContent(input: {
194+
codexBin: string;
195+
workspaceRoot: string;
196+
}): string {
197+
// CRLF line endings are mandatory - LF-only .bat files misbehave on
198+
// some cmd.exe versions and we enforce CRLF everywhere else in the
199+
// repo via .gitattributes.
200+
const lines = [
201+
"@echo off",
202+
`cd /d ${winQuote(input.workspaceRoot)}`,
203+
`${winQuote(input.codexBin)} -c "model_reasoning_effort=high" login`,
204+
"echo.",
205+
"echo Codex login flow finished. You can close this window.",
206+
"pause"
207+
];
208+
return lines.join("\r\n") + "\r\n";
209+
}
210+
172211
export function buildCodexLoginCommandWindows(input: {
173212
codexBin: string;
174213
workspaceRoot: string;

core/agent/test/codex-login.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import test from "node:test";
22
import assert from "node:assert/strict";
33
import { EventEmitter } from "node:events";
44
import { PassThrough } from "node:stream";
5-
import { buildCodexLoginCommand, buildCodexLoginCommandWindows, parseCodexLoginStatus, readCodexLoginStatus } from "../src/codex-login.js";
5+
import { buildCodexLoginBatContent, buildCodexLoginCommand, buildCodexLoginCommandWindows, parseCodexLoginStatus, readCodexLoginStatus } from "../src/codex-login.js";
66
import type { SpawnedProcessLike } from "../src/codex.js";
77

88
class FakeProcess extends EventEmitter implements SpawnedProcessLike {
@@ -43,6 +43,32 @@ test("buildCodexLoginCommandWindows uses cd /d and double-quote escaping", () =>
4343
assert.match(command, /login$/);
4444
});
4545

46+
test("buildCodexLoginBatContent writes an ASCII bat with CRLF line endings", () => {
47+
const content = buildCodexLoginBatContent({
48+
codexBin: "C:\\Program Files\\codex\\codex.exe",
49+
workspaceRoot: "C:\\Users\\test\\my repo"
50+
});
51+
52+
// CRLF everywhere, LF only as part of CRLF
53+
assert.match(content, /\r\n/, "bat content must use CRLF");
54+
for (let i = 0; i < content.length; i += 1) {
55+
if (content.charCodeAt(i) === 0x0a && content.charCodeAt(i - 1) !== 0x0d) {
56+
assert.fail("bat content must not contain bare LF line endings");
57+
}
58+
}
59+
60+
// Pure ASCII - zh-CN cmd.exe code page 936 cannot decode UTF-8 multi-byte sequences.
61+
for (let i = 0; i < content.length; i += 1) {
62+
const code = content.charCodeAt(i);
63+
assert.ok(code <= 0x7f, `non-ASCII byte 0x${code.toString(16)} at offset ${i}`);
64+
}
65+
66+
assert.match(content, /@echo off\r\n/);
67+
assert.match(content, /cd \/d "C:\\Users\\test\\my repo"\r\n/);
68+
assert.match(content, /"C:\\Program Files\\codex\\codex.exe" -c "model_reasoning_effort=high" login\r\n/);
69+
assert.match(content, /\r\npause\r\n$/);
70+
});
71+
4672
test("readCodexLoginStatus uses CLI status output", async () => {
4773
const child = new FakeProcess();
4874

0 commit comments

Comments
 (0)