mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 03:15:46 -06:00
fix(tui): stop auth child on exit (#126476)
This commit is contained in:
committed by
GitHub
parent
c5fd6252a7
commit
13fd888281
@@ -0,0 +1,107 @@
|
||||
import { chmod, mkdtemp, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { sleep } from "../utils/sleep.js";
|
||||
import {
|
||||
disposeActiveTuiFixtures,
|
||||
startTuiFixture,
|
||||
} from "./tui-pty-harness-fixture-test-support.js";
|
||||
|
||||
const STARTUP_TIMEOUT_MS = 60_000;
|
||||
const EXIT_TIMEOUT_MS = 4_000;
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
async function createCodexFixture(exitMs?: number) {
|
||||
const dir = await mkdtemp(path.join(tmpdir(), "openclaw-tui-auth-"));
|
||||
tempDirs.push(dir);
|
||||
const scriptPath = path.join(dir, "codex-fixture.cjs");
|
||||
await writeFile(
|
||||
scriptPath,
|
||||
[
|
||||
'console.log("AUTH_CHILD_STARTED:" + process.pid);',
|
||||
exitMs === undefined
|
||||
? "setInterval(() => {}, 1000);"
|
||||
: `setTimeout(() => process.exit(0), ${String(exitMs)});`,
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
);
|
||||
if (process.platform === "win32") {
|
||||
await writeFile(
|
||||
path.join(dir, "codex.cmd"),
|
||||
`@"${process.execPath}" "${scriptPath}" %*\r\n`,
|
||||
"utf8",
|
||||
);
|
||||
} else {
|
||||
const launcherPath = path.join(dir, "codex");
|
||||
await writeFile(launcherPath, `#!/bin/sh\nexec "${process.execPath}" "${scriptPath}" "$@"\n`);
|
||||
await chmod(launcherPath, 0o755);
|
||||
}
|
||||
return {
|
||||
pathEnv: `${dir}${path.delimiter}${process.env.PATH ?? ""}`,
|
||||
};
|
||||
}
|
||||
|
||||
async function waitForProcessExit(pid: number, timeoutMs: number): Promise<void> {
|
||||
const deadline = Date.now() + timeoutMs;
|
||||
while (Date.now() < deadline) {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
await sleep(10);
|
||||
}
|
||||
throw new Error(`auth child ${String(pid)} remained alive`);
|
||||
}
|
||||
|
||||
describe.sequential("TUI auth child lifecycle", () => {
|
||||
afterEach(async () => {
|
||||
await disposeActiveTuiFixtures();
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"terminates the foreground auth child when SIGTERM exits the TUI",
|
||||
async () => {
|
||||
const auth = await createCodexFixture();
|
||||
const fixture = await startTuiFixture({
|
||||
env: {
|
||||
PATH: auth.pathEnv,
|
||||
},
|
||||
});
|
||||
await fixture.run.waitForOutput("local ready", STARTUP_TIMEOUT_MS);
|
||||
await fixture.run.write("/auth openai\r", { delay: false });
|
||||
await fixture.run.waitForOutput("AUTH_CHILD_STARTED:", STARTUP_TIMEOUT_MS);
|
||||
const pidMatch = fixture.run.visibleOutput().match(/AUTH_CHILD_STARTED:(\d+)/u);
|
||||
expect(pidMatch).not.toBeNull();
|
||||
const authPid = Number(pidMatch?.[1]);
|
||||
|
||||
process.kill(fixture.run.pid, "SIGTERM");
|
||||
|
||||
await expect(fixture.run.waitForExit(EXIT_TIMEOUT_MS)).resolves.toBeDefined();
|
||||
await expect(waitForProcessExit(authPid, 750)).resolves.toBeUndefined();
|
||||
},
|
||||
STARTUP_TIMEOUT_MS + EXIT_TIMEOUT_MS,
|
||||
);
|
||||
|
||||
it(
|
||||
"resumes the TUI after normal auth completion",
|
||||
async () => {
|
||||
const auth = await createCodexFixture(50);
|
||||
const fixture = await startTuiFixture({
|
||||
env: {
|
||||
PATH: auth.pathEnv,
|
||||
},
|
||||
});
|
||||
await fixture.run.waitForOutput("local ready", STARTUP_TIMEOUT_MS);
|
||||
await fixture.run.write("/auth openai\r", { delay: false });
|
||||
await fixture.run.waitForOutput("auth flow finished for openai", STARTUP_TIMEOUT_MS);
|
||||
await fixture.run.write("/gateway-status\r", { delay: false });
|
||||
await fixture.run.waitForOutput("fixture gateway ok", STARTUP_TIMEOUT_MS);
|
||||
},
|
||||
STARTUP_TIMEOUT_MS * 2,
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,98 @@
|
||||
import type { ChildProcess } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const killTreeMocks = vi.hoisted(() => ({ signalProcessTree: vi.fn() }));
|
||||
|
||||
vi.mock("../process/kill-tree.js", () => ({
|
||||
signalProcessTree: killTreeMocks.signalProcessTree,
|
||||
}));
|
||||
|
||||
import { createTuiAuthChildOwner } from "./tui-auth-child.js";
|
||||
|
||||
function createChild(pid: number): ChildProcess {
|
||||
return Object.assign(new EventEmitter(), {
|
||||
exitCode: null,
|
||||
kill: vi.fn(),
|
||||
pid,
|
||||
signalCode: null,
|
||||
}) as unknown as ChildProcess;
|
||||
}
|
||||
|
||||
function emitExit(
|
||||
child: ChildProcess,
|
||||
exitCode: number | null,
|
||||
signal: NodeJS.Signals | null,
|
||||
): void {
|
||||
Object.assign(child, { exitCode, signalCode: signal });
|
||||
child.emit("exit", exitCode, signal);
|
||||
}
|
||||
|
||||
describe("TUI auth child owner", () => {
|
||||
afterEach(() => {
|
||||
killTreeMocks.signalProcessTree.mockReset();
|
||||
});
|
||||
|
||||
it("does not spawn after terminal close", async () => {
|
||||
const owner = createTuiAuthChildOwner();
|
||||
const spawnChild = vi.fn(() => createChild(101));
|
||||
owner.close();
|
||||
|
||||
await expect(owner.spawnAndWait(spawnChild)).rejects.toThrow("owner is closed");
|
||||
expect(spawnChild).not.toHaveBeenCalled();
|
||||
expect(owner.running).toBe(false);
|
||||
});
|
||||
|
||||
it("waits for normal completion and releases the exact child", async () => {
|
||||
const owner = createTuiAuthChildOwner();
|
||||
const child = createChild(102);
|
||||
const result = owner.spawnAndWait(() => child);
|
||||
expect(owner.running).toBe(true);
|
||||
const secondSpawn = vi.fn(() => createChild(202));
|
||||
await expect(owner.spawnAndWait(secondSpawn)).rejects.toThrow("already running");
|
||||
expect(secondSpawn).not.toHaveBeenCalled();
|
||||
|
||||
emitExit(child, 0, null);
|
||||
|
||||
await expect(result).resolves.toEqual({ exitCode: 0, signal: null });
|
||||
expect(owner.running).toBe(false);
|
||||
expect(killTreeMocks.signalProcessTree).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("cancels gracefully, then force-kills only the still-active child", async () => {
|
||||
const owner = createTuiAuthChildOwner();
|
||||
const child = createChild(103);
|
||||
const result = owner.spawnAndWait(() => child);
|
||||
|
||||
owner.close();
|
||||
owner.close();
|
||||
|
||||
expect(killTreeMocks.signalProcessTree).toHaveBeenCalledTimes(1);
|
||||
expect(killTreeMocks.signalProcessTree).toHaveBeenCalledWith(103, "SIGTERM", {
|
||||
detached: false,
|
||||
});
|
||||
await new Promise((resolve) => {
|
||||
setTimeout(resolve, 1_050);
|
||||
});
|
||||
expect(killTreeMocks.signalProcessTree).toHaveBeenLastCalledWith(103, "SIGKILL", {
|
||||
detached: false,
|
||||
});
|
||||
|
||||
emitExit(child, null, "SIGKILL");
|
||||
await expect(result).resolves.toEqual({ exitCode: null, signal: "SIGKILL" });
|
||||
});
|
||||
|
||||
it("clears force escalation when the owned child exits", async () => {
|
||||
const owner = createTuiAuthChildOwner();
|
||||
const child = createChild(104);
|
||||
const result = owner.spawnAndWait(() => child);
|
||||
owner.close();
|
||||
emitExit(child, null, "SIGTERM");
|
||||
|
||||
await expect(result).resolves.toEqual({ exitCode: null, signal: "SIGTERM" });
|
||||
await new Promise((resolve) => {
|
||||
setTimeout(resolve, 1_050);
|
||||
});
|
||||
expect(killTreeMocks.signalProcessTree).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,95 @@
|
||||
import type { ChildProcess } from "node:child_process";
|
||||
import { signalProcessTree } from "../process/kill-tree.js";
|
||||
|
||||
const AUTH_CHILD_FORCE_EXIT_MS = 1_000;
|
||||
|
||||
type TuiAuthChildResult = {
|
||||
exitCode: number | null;
|
||||
signal: NodeJS.Signals | null;
|
||||
};
|
||||
|
||||
type ActiveAuthChild = {
|
||||
child: ChildProcess;
|
||||
forceTimer?: ReturnType<typeof setTimeout>;
|
||||
};
|
||||
|
||||
function isChildRunning(child: ChildProcess): boolean {
|
||||
return child.exitCode === null && child.signalCode === null;
|
||||
}
|
||||
|
||||
function signalAuthChild(child: ChildProcess, signal: "SIGTERM" | "SIGKILL"): void {
|
||||
if (child.pid) {
|
||||
// Auth shares the TUI foreground process group. POSIX must target only this PID;
|
||||
// Windows uses taskkill /T here so a cmd.exe launcher cannot strand its CLI child.
|
||||
signalProcessTree(child.pid, signal, { detached: false });
|
||||
return;
|
||||
}
|
||||
child.kill(signal);
|
||||
}
|
||||
|
||||
export function createTuiAuthChildOwner() {
|
||||
let active: ActiveAuthChild | null = null;
|
||||
let closed = false;
|
||||
|
||||
const clearActive = (owned: ActiveAuthChild): void => {
|
||||
if (owned.forceTimer) {
|
||||
clearTimeout(owned.forceTimer);
|
||||
owned.forceTimer = undefined;
|
||||
}
|
||||
if (active === owned) {
|
||||
active = null;
|
||||
}
|
||||
};
|
||||
|
||||
const cancel = (owned: ActiveAuthChild): void => {
|
||||
if (!isChildRunning(owned.child)) {
|
||||
return;
|
||||
}
|
||||
signalAuthChild(owned.child, "SIGTERM");
|
||||
owned.forceTimer = setTimeout(() => {
|
||||
if (active !== owned || !isChildRunning(owned.child)) {
|
||||
return;
|
||||
}
|
||||
signalAuthChild(owned.child, "SIGKILL");
|
||||
}, AUTH_CHILD_FORCE_EXIT_MS);
|
||||
owned.forceTimer.unref?.();
|
||||
};
|
||||
|
||||
return {
|
||||
get running(): boolean {
|
||||
return active !== null && isChildRunning(active.child);
|
||||
},
|
||||
spawnAndWait: async (spawnChild: () => ChildProcess): Promise<TuiAuthChildResult> => {
|
||||
if (closed) {
|
||||
throw new Error("TUI auth child owner is closed");
|
||||
}
|
||||
if (active) {
|
||||
throw new Error("TUI auth child is already running");
|
||||
}
|
||||
const owned: ActiveAuthChild = { child: spawnChild() };
|
||||
active = owned;
|
||||
return await new Promise<TuiAuthChildResult>((resolve, reject) => {
|
||||
let settled = false;
|
||||
const settle = (complete: () => void): void => {
|
||||
if (settled) {
|
||||
return;
|
||||
}
|
||||
settled = true;
|
||||
clearActive(owned);
|
||||
complete();
|
||||
};
|
||||
owned.child.once("error", (error) => settle(() => reject(error)));
|
||||
owned.child.once("exit", (exitCode, signal) => settle(() => resolve({ exitCode, signal })));
|
||||
});
|
||||
},
|
||||
close: (): void => {
|
||||
if (closed) {
|
||||
return;
|
||||
}
|
||||
closed = true;
|
||||
if (active) {
|
||||
cancel(active);
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -124,6 +124,7 @@ describe("local TUI PTY fixture support", () => {
|
||||
const run = {
|
||||
cols: 100,
|
||||
output: () => output,
|
||||
pid: 123,
|
||||
rows: 30,
|
||||
visibleOutput: () => output.replace(/\s+/gu, " "),
|
||||
write: async (data: string) => {
|
||||
|
||||
@@ -15,6 +15,7 @@ type PtyExitEvent = Parameters<Parameters<IPty["onExit"]>[0]>[0];
|
||||
export type PtyRun = {
|
||||
cols: number;
|
||||
output: () => string;
|
||||
pid: number;
|
||||
rows: number;
|
||||
visibleOutput: () => string;
|
||||
write: (data: string, opts?: { delay?: boolean }) => Promise<void>;
|
||||
@@ -370,6 +371,7 @@ export function startPty(
|
||||
const run: PtyRun = {
|
||||
cols,
|
||||
output: () => output,
|
||||
pid: pty.pid,
|
||||
rows,
|
||||
visibleOutput: () => visibleOutput,
|
||||
write: async (data, writeOpts) => await writePtyInput(pty, data, ptyEnv, writeOpts),
|
||||
|
||||
+31
-32
@@ -49,6 +49,7 @@ import { ChatLog } from "./components/chat-log.js";
|
||||
import { CustomEditor } from "./components/custom-editor.js";
|
||||
import { resolveLocalRunShutdownGraceMs } from "./local-run-shutdown.js";
|
||||
import { editorTheme, tuiTheme as theme } from "./theme/theme.js";
|
||||
import { createTuiAuthChildOwner } from "./tui-auth-child.js";
|
||||
import { sanitizeAutocompleteProvider } from "./tui-autocomplete.js";
|
||||
import type { TuiBackend } from "./tui-backend.js";
|
||||
import { createCommandHandlers } from "./tui-command-handlers.js";
|
||||
@@ -800,6 +801,7 @@ async function runTuiUnlocked(opts: RunTuiOptions): Promise<TuiResult> {
|
||||
let dynamicSlashCommandsRefreshTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
let exitRequested = false;
|
||||
let exitResult: TuiResult = { exitReason: "exit" };
|
||||
const authChild = createTuiAuthChildOwner();
|
||||
let statusTimer: NodeJS.Timeout | null = null;
|
||||
let statusStartedAt: number | null = null;
|
||||
let lastActivityStatus = "idle";
|
||||
@@ -1319,14 +1321,16 @@ async function runTuiUnlocked(opts: RunTuiOptions): Promise<TuiResult> {
|
||||
try {
|
||||
return await work();
|
||||
} finally {
|
||||
if (isLocalMode) {
|
||||
setConsoleSubsystemFilter(["__openclaw_tui_quiet__"]);
|
||||
if (!exitRequested) {
|
||||
if (isLocalMode) {
|
||||
setConsoleSubsystemFilter(["__openclaw_tui_quiet__"]);
|
||||
}
|
||||
tui.start();
|
||||
tui.setFocus(editor);
|
||||
updateHeader();
|
||||
updateFooter();
|
||||
tui.requestRender(true);
|
||||
}
|
||||
tui.start();
|
||||
tui.setFocus(editor);
|
||||
updateHeader();
|
||||
updateFooter();
|
||||
tui.requestRender(true);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1342,32 +1346,26 @@ async function runTuiUnlocked(opts: RunTuiOptions): Promise<TuiResult> {
|
||||
? await resolveCodexCliBin()
|
||||
: null;
|
||||
|
||||
return await new Promise<{ exitCode: number | null; signal: NodeJS.Signals | null }>(
|
||||
(resolve, reject) => {
|
||||
let command: string;
|
||||
let args: string[];
|
||||
let cwd: string;
|
||||
if (codexBin) {
|
||||
command = codexBin;
|
||||
args = ["login"];
|
||||
cwd = resolveUsableCwd();
|
||||
} else {
|
||||
const invocation = resolveTuiLocalAuthCliInvocation({ provider });
|
||||
({ command, args, cwd } = invocation);
|
||||
}
|
||||
let command: string;
|
||||
let args: string[];
|
||||
let cwd: string;
|
||||
if (codexBin) {
|
||||
command = codexBin;
|
||||
args = ["login"];
|
||||
cwd = resolveUsableCwd();
|
||||
} else {
|
||||
const invocation = resolveTuiLocalAuthCliInvocation({ provider });
|
||||
({ command, args, cwd } = invocation);
|
||||
}
|
||||
|
||||
const invocation = resolveLocalAuthSpawnInvocation({ command, args });
|
||||
const child = spawn(invocation.command, invocation.args, {
|
||||
cwd,
|
||||
env: process.env,
|
||||
stdio: "inherit",
|
||||
...invocation.options,
|
||||
});
|
||||
child.once("error", reject);
|
||||
child.once("exit", (exitCode, signal) => {
|
||||
resolve({ exitCode, signal });
|
||||
});
|
||||
},
|
||||
const invocation = resolveLocalAuthSpawnInvocation({ command, args });
|
||||
return await authChild.spawnAndWait(() =>
|
||||
spawn(invocation.command, invocation.args, {
|
||||
cwd,
|
||||
env: process.env,
|
||||
stdio: "inherit",
|
||||
...invocation.options,
|
||||
}),
|
||||
);
|
||||
})
|
||||
: undefined;
|
||||
@@ -1521,6 +1519,7 @@ async function runTuiUnlocked(opts: RunTuiOptions): Promise<TuiResult> {
|
||||
return;
|
||||
}
|
||||
exitRequested = true;
|
||||
authChild.close();
|
||||
// Exit owns the input boundary before transport teardown can race a buffered submit.
|
||||
disposeSubmitBurst();
|
||||
connectionGeneration += 1;
|
||||
|
||||
@@ -5,11 +5,13 @@ import { resolveRepoRootPath, sharedVitestConfig } from "./vitest.shared.config.
|
||||
|
||||
const targetableIncludes = [
|
||||
"src/tui/tui-pty-harness-assertion-test-support.test.ts",
|
||||
"src/tui/tui-auth-child-pty.e2e.test.ts",
|
||||
"src/tui/tui-pty-harness.e2e.test.ts",
|
||||
"src/tui/tui-session-identity-pty.e2e.test.ts",
|
||||
"src/tui/tui-pty-local.e2e.test.ts",
|
||||
"src/tui/tui-reset-transition-pty.e2e.test.ts",
|
||||
"tui/tui-pty-harness-assertion-test-support.test.ts",
|
||||
"tui/tui-auth-child-pty.e2e.test.ts",
|
||||
"tui/tui-pty-harness.e2e.test.ts",
|
||||
"tui/tui-session-identity-pty.e2e.test.ts",
|
||||
"tui/tui-pty-local.e2e.test.ts",
|
||||
@@ -26,6 +28,7 @@ function createTuiPtyVitestConfig(env?: Record<string, string | undefined>) {
|
||||
const configEnv = env ?? process.env;
|
||||
const includeLocal = configEnv.OPENCLAW_TUI_PTY_INCLUDE_LOCAL === "1";
|
||||
const include = [
|
||||
"tui/tui-auth-child-pty.e2e.test.ts",
|
||||
"tui/tui-pty-harness.e2e.test.ts",
|
||||
"tui/tui-session-identity-pty.e2e.test.ts",
|
||||
"tui/tui-reset-transition-pty.e2e.test.ts",
|
||||
|
||||
Reference in New Issue
Block a user