diff --git a/src/tui/tui-auth-child-pty.e2e.test.ts b/src/tui/tui-auth-child-pty.e2e.test.ts new file mode 100644 index 000000000000..a1ebeab2bec9 --- /dev/null +++ b/src/tui/tui-auth-child-pty.e2e.test.ts @@ -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 { + 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, + ); +}); diff --git a/src/tui/tui-auth-child.test.ts b/src/tui/tui-auth-child.test.ts new file mode 100644 index 000000000000..d7bcd79f31d6 --- /dev/null +++ b/src/tui/tui-auth-child.test.ts @@ -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); + }); +}); diff --git a/src/tui/tui-auth-child.ts b/src/tui/tui-auth-child.ts new file mode 100644 index 000000000000..2d19a8b0c644 --- /dev/null +++ b/src/tui/tui-auth-child.ts @@ -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; +}; + +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 => { + 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((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); + } + }, + }; +} diff --git a/src/tui/tui-pty-local-test-support.test.ts b/src/tui/tui-pty-local-test-support.test.ts index eac5c93b1a41..fb7a9e559dfb 100644 --- a/src/tui/tui-pty-local-test-support.test.ts +++ b/src/tui/tui-pty-local-test-support.test.ts @@ -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) => { diff --git a/src/tui/tui-pty-test-support.ts b/src/tui/tui-pty-test-support.ts index 4f5f6707a42b..0e956ca649dd 100644 --- a/src/tui/tui-pty-test-support.ts +++ b/src/tui/tui-pty-test-support.ts @@ -15,6 +15,7 @@ type PtyExitEvent = Parameters[0]>[0]; export type PtyRun = { cols: number; output: () => string; + pid: number; rows: number; visibleOutput: () => string; write: (data: string, opts?: { delay?: boolean }) => Promise; @@ -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), diff --git a/src/tui/tui.ts b/src/tui/tui.ts index fe7bbd0a734f..7598a5a2be51 100644 --- a/src/tui/tui.ts +++ b/src/tui/tui.ts @@ -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 { let dynamicSlashCommandsRefreshTimer: ReturnType | 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 { 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 { ? 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 { return; } exitRequested = true; + authChild.close(); // Exit owns the input boundary before transport teardown can race a buffered submit. disposeSubmitBurst(); connectionGeneration += 1; diff --git a/test/vitest/vitest.tui-pty.config.ts b/test/vitest/vitest.tui-pty.config.ts index aa81c60a77d4..d6885cc447ec 100644 --- a/test/vitest/vitest.tui-pty.config.ts +++ b/test/vitest/vitest.tui-pty.config.ts @@ -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) { 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",