mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(tui): stop Codex login lookup hanging the terminal (#109452)
* fix(tui): bound Codex CLI lookup * fix(tui): bound Codex CLI lookup asynchronously Co-authored-by: 杨浩宇0668001029 <yang.haoyu@xydigit.com> * style(tui): format Codex auth flow Co-authored-by: 杨浩宇0668001029 <yang.haoyu@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
co-authored by
Peter Steinberger
parent
cc920838bc
commit
5360c75de2
@@ -1,29 +1,50 @@
|
||||
// Covers TUI Codex CLI lookup command selection.
|
||||
// Covers bounded TUI Codex CLI lookup command selection.
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { withMockedWindowsPlatform } from "../test-utils/vitest-spies.js";
|
||||
import { withMockedPlatform, withMockedWindowsPlatform } from "../test-utils/vitest-spies.js";
|
||||
|
||||
const execFileSyncMock = vi.hoisted(() => vi.fn());
|
||||
const runCommandWithTimeoutMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("node:child_process", async () => {
|
||||
const actual = await vi.importActual<typeof import("node:child_process")>("node:child_process");
|
||||
return {
|
||||
...actual,
|
||||
execFileSync: execFileSyncMock,
|
||||
};
|
||||
});
|
||||
vi.mock("../process/exec.js", () => ({ runCommandWithTimeout: runCommandWithTimeoutMock }));
|
||||
|
||||
import { resolveCodexCliBin } from "./tui.js";
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
vi.unstubAllEnvs();
|
||||
execFileSyncMock.mockReset();
|
||||
runCommandWithTimeoutMock.mockReset();
|
||||
});
|
||||
|
||||
describe("resolveCodexCliBin", () => {
|
||||
it("uses the trusted Windows where.exe when resolving codex", async () => {
|
||||
it("bounds lookup and returns the first PATH match", async () => {
|
||||
runCommandWithTimeoutMock.mockResolvedValue({
|
||||
code: 0,
|
||||
stdout: "/usr/local/bin/codex\n/opt/bin/codex\n",
|
||||
termination: "exit",
|
||||
});
|
||||
|
||||
await withMockedPlatform("linux", async () => {
|
||||
await expect(resolveCodexCliBin()).resolves.toBe("/usr/local/bin/codex");
|
||||
});
|
||||
expect(runCommandWithTimeoutMock).toHaveBeenCalledWith(["which", "codex"], {
|
||||
killSignal: "SIGKILL",
|
||||
maxOutputBytes: 64 * 1024,
|
||||
timeoutMs: 5_000,
|
||||
});
|
||||
});
|
||||
|
||||
it("returns null when lookup times out", async () => {
|
||||
runCommandWithTimeoutMock.mockResolvedValue({
|
||||
code: null,
|
||||
stdout: "",
|
||||
termination: "timeout",
|
||||
});
|
||||
|
||||
await expect(resolveCodexCliBin()).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it("uses the trusted Windows where.exe", async () => {
|
||||
const accessSync = fs.accessSync.bind(fs);
|
||||
vi.spyOn(fs, "accessSync").mockImplementation((filePath, mode) => {
|
||||
if (String(filePath).toLowerCase() === "c:\\windows\\system32\\reg.exe") {
|
||||
@@ -32,16 +53,22 @@ describe("resolveCodexCliBin", () => {
|
||||
return accessSync(filePath, mode);
|
||||
});
|
||||
vi.stubEnv("SystemRoot", "D:\\Windows");
|
||||
execFileSyncMock.mockReturnValue("D:\\Tools\\codex.exe\r\n");
|
||||
|
||||
await withMockedWindowsPlatform(async () => {
|
||||
expect(resolveCodexCliBin()).toBe("D:\\Tools\\codex.exe");
|
||||
runCommandWithTimeoutMock.mockResolvedValue({
|
||||
code: 0,
|
||||
stdout: "D:\\Tools\\codex.exe\r\n",
|
||||
termination: "exit",
|
||||
});
|
||||
|
||||
expect(execFileSyncMock).toHaveBeenCalledWith(
|
||||
path.win32.join("D:\\Windows", "System32", "where.exe"),
|
||||
["codex"],
|
||||
{ encoding: "utf8" },
|
||||
await withMockedWindowsPlatform(async () => {
|
||||
await expect(resolveCodexCliBin()).resolves.toBe("D:\\Tools\\codex.exe");
|
||||
});
|
||||
expect(runCommandWithTimeoutMock).toHaveBeenCalledWith(
|
||||
[path.win32.join("D:\\Windows", "System32", "where.exe"), "codex"],
|
||||
{
|
||||
killSignal: "SIGKILL",
|
||||
maxOutputBytes: 64 * 1024,
|
||||
timeoutMs: 5_000,
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
+4
-4
@@ -583,8 +583,8 @@ describe("TUI shutdown safety", () => {
|
||||
});
|
||||
|
||||
describe("resolveCodexCliBin", () => {
|
||||
it("returns a string path when codex CLI is installed", () => {
|
||||
const result = resolveCodexCliBin();
|
||||
it("returns a string path when codex CLI is installed", async () => {
|
||||
const result = await resolveCodexCliBin();
|
||||
// In this test environment codex is installed; verify it returns a non-empty path
|
||||
if (result !== null) {
|
||||
expect(typeof result).toBe("string");
|
||||
@@ -593,8 +593,8 @@ describe("resolveCodexCliBin", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("returns null or a valid path (never throws)", () => {
|
||||
const result = resolveCodexCliBin();
|
||||
it("returns null or a valid path (never throws)", async () => {
|
||||
const result = await resolveCodexCliBin();
|
||||
if (result === null) {
|
||||
expect(result).toBeNull();
|
||||
} else {
|
||||
|
||||
+50
-42
@@ -1,5 +1,5 @@
|
||||
// Runs the interactive TUI loop and coordinates backend, input, and rendering.
|
||||
import { execFileSync, spawn } from "node:child_process";
|
||||
import { spawn } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
@@ -23,6 +23,7 @@ import { registerUncaughtExceptionHandler } from "../infra/unhandled-rejections.
|
||||
import { getWindowsSystem32ExePath } from "../infra/windows-install-roots.js";
|
||||
import { setConsoleSubsystemFilter } from "../logging/console.js";
|
||||
import { loggingState } from "../logging/state.js";
|
||||
import { runCommandWithTimeout } from "../process/exec.js";
|
||||
import {
|
||||
buildWindowsCmdExeCommandLine,
|
||||
isWindowsBatchCommand,
|
||||
@@ -102,6 +103,7 @@ const OPENCLAW_DIST_ENTRY_MJS_PATH = fileURLToPath(
|
||||
);
|
||||
|
||||
const OPENAI_CODEX_PROVIDER = "openai";
|
||||
const CODEX_CLI_LOOKUP_TIMEOUT_MS = 5_000;
|
||||
|
||||
type RunTuiOptions = TuiOptions & {
|
||||
backend?: TuiBackend;
|
||||
@@ -117,13 +119,20 @@ type RunTuiOptions = TuiOptions & {
|
||||
};
|
||||
|
||||
/** Resolve the absolute path to the `codex` CLI binary, or `null` if not installed. */
|
||||
export function resolveCodexCliBin(): string | null {
|
||||
export async function resolveCodexCliBin(): Promise<string | null> {
|
||||
const lookupCommand =
|
||||
process.platform === "win32" ? getWindowsSystem32ExePath("where.exe") : "which";
|
||||
try {
|
||||
const lookupCmd =
|
||||
process.platform === "win32" ? getWindowsSystem32ExePath("where.exe") : "which";
|
||||
// `where` on Windows can return multiple lines; take the first match.
|
||||
const raw = execFileSync(lookupCmd, ["codex"], { encoding: "utf8" }).trim();
|
||||
return raw.split(/\r?\n/)[0] || null;
|
||||
const result = await runCommandWithTimeout([lookupCommand, "codex"], {
|
||||
killSignal: "SIGKILL",
|
||||
maxOutputBytes: 64 * 1024,
|
||||
timeoutMs: CODEX_CLI_LOOKUP_TIMEOUT_MS,
|
||||
});
|
||||
if (result.code !== 0 || result.termination !== "exit") {
|
||||
return null;
|
||||
}
|
||||
// `where` on Windows can return multiple matches; use PATH order.
|
||||
return result.stdout.trim().split(/\r?\n/)[0]?.trim() || null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
@@ -1183,45 +1192,44 @@ export async function runTui(opts: RunTuiOptions): Promise<TuiResult> {
|
||||
|
||||
const runAuthFlow = isLocalMode
|
||||
? async (params: { provider?: string }) =>
|
||||
await withTuiSuspended(
|
||||
async () =>
|
||||
await new Promise<{ exitCode: number | null; signal: NodeJS.Signals | null }>(
|
||||
(resolve, reject) => {
|
||||
const provider = params.provider?.trim() || undefined;
|
||||
await withTuiSuspended(async () => {
|
||||
const provider = params.provider?.trim() || undefined;
|
||||
|
||||
// Codex owns its auth store; delegate when the CLI is available.
|
||||
const codexBin =
|
||||
provider === OPENAI_CODEX_PROVIDER ||
|
||||
(!provider && sessionInfo.modelProvider === OPENAI_CODEX_PROVIDER)
|
||||
? resolveCodexCliBin()
|
||||
: null;
|
||||
// Codex owns its auth store; delegate when the CLI is available.
|
||||
const codexBin =
|
||||
provider === OPENAI_CODEX_PROVIDER ||
|
||||
(!provider && sessionInfo.modelProvider === OPENAI_CODEX_PROVIDER)
|
||||
? await resolveCodexCliBin()
|
||||
: null;
|
||||
|
||||
let command: string;
|
||||
let args: string[];
|
||||
if (codexBin) {
|
||||
command = codexBin;
|
||||
args = ["login"];
|
||||
} else {
|
||||
({ command, args } = resolveLocalAuthCliInvocation());
|
||||
if (provider) {
|
||||
args.push("--provider", provider);
|
||||
}
|
||||
return await new Promise<{ exitCode: number | null; signal: NodeJS.Signals | null }>(
|
||||
(resolve, reject) => {
|
||||
let command: string;
|
||||
let args: string[];
|
||||
if (codexBin) {
|
||||
command = codexBin;
|
||||
args = ["login"];
|
||||
} else {
|
||||
({ command, args } = resolveLocalAuthCliInvocation());
|
||||
if (provider) {
|
||||
args.push("--provider", provider);
|
||||
}
|
||||
}
|
||||
|
||||
const invocation = resolveLocalAuthSpawnInvocation({ command, args });
|
||||
const child = spawn(invocation.command, invocation.args, {
|
||||
cwd: resolveLocalAuthSpawnCwd({ args, defaultCwd: resolveUsableCwd() }),
|
||||
env: process.env,
|
||||
stdio: "inherit",
|
||||
...invocation.options,
|
||||
});
|
||||
child.once("error", reject);
|
||||
child.once("exit", (exitCode, signal) => {
|
||||
resolve({ exitCode, signal });
|
||||
});
|
||||
},
|
||||
),
|
||||
)
|
||||
const invocation = resolveLocalAuthSpawnInvocation({ command, args });
|
||||
const child = spawn(invocation.command, invocation.args, {
|
||||
cwd: resolveLocalAuthSpawnCwd({ args, defaultCwd: resolveUsableCwd() }),
|
||||
env: process.env,
|
||||
stdio: "inherit",
|
||||
...invocation.options,
|
||||
});
|
||||
child.once("error", reject);
|
||||
child.once("exit", (exitCode, signal) => {
|
||||
resolve({ exitCode, signal });
|
||||
});
|
||||
},
|
||||
);
|
||||
})
|
||||
: undefined;
|
||||
|
||||
const updateFooter = () => {
|
||||
|
||||
Reference in New Issue
Block a user