From cd96542d372e0cff0191c7fc0267983a124b9611 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 25 May 2026 23:46:12 +0200 Subject: [PATCH] fix(test): harden macos onboarding e2e --- scripts/e2e/lib/onboard/scenario.sh | 51 +++++++++++--- .../lib/release-typed-onboarding/scenario.sh | 14 +++- scripts/e2e/lib/run-with-pty.mjs | 49 +++++++++++++ scripts/lib/openclaw-e2e-instance.sh | 2 + scripts/lib/openclaw-test-state.mjs | 4 ++ test/scripts/e2e-run-with-pty.test.ts | 68 +++++++++++++++++++ test/scripts/e2e-shell-tempfiles.test.ts | 41 +++++++++++ test/scripts/openclaw-test-state.test.ts | 24 +++++++ 8 files changed, 240 insertions(+), 13 deletions(-) create mode 100644 scripts/e2e/lib/run-with-pty.mjs create mode 100644 test/scripts/e2e-run-with-pty.test.ts create mode 100644 test/scripts/e2e-shell-tempfiles.test.ts diff --git a/scripts/e2e/lib/onboard/scenario.sh b/scripts/e2e/lib/onboard/scenario.sh index c4c705775f5f..9bc2f133f0a4 100644 --- a/scripts/e2e/lib/onboard/scenario.sh +++ b/scripts/e2e/lib/onboard/scenario.sh @@ -80,33 +80,62 @@ run_wizard_cmd() { local send_fn="$4" local with_gateway="${5:-false}" local validate_fn="${6:-}" + local input_fifo_dir="" + local input_fifo="" + local wizard_pid="" + local gw_pid="" + local wizard_status=0 echo "== Wizard case: $case_name ==" set_isolated_openclaw_env "$state_ref" - input_fifo="$(mktemp -u "/tmp/openclaw-onboard-${case_name}.XXXXXX")" - mkfifo "$input_fifo" + input_fifo_dir="$(mktemp -d "/tmp/openclaw-onboard-${case_name}.XXXXXX")" + input_fifo="$input_fifo_dir/stdin.fifo" + if ! mkfifo "$input_fifo"; then + rm -rf "$input_fifo_dir" + return 1 + fi local log_path="/tmp/openclaw-onboard-${case_name}.log" WIZARD_LOG_PATH="$log_path" export WIZARD_LOG_PATH # Run under script to keep an interactive TTY for clack prompts. openclaw_e2e_run_script_with_pty "$command" "$log_path" <"$input_fifo" >/dev/null 2>&1 & wizard_pid=$! - exec 3>"$input_fifo" + if ! exec 3>"$input_fifo"; then + openclaw_e2e_stop_process "$wizard_pid" + rm -rf "$input_fifo_dir" + return 1 + fi - local gw_pid="" if [ "$with_gateway" = "true" ]; then start_gateway gw_pid="$GATEWAY_PID" - wait_for_gateway + if ! wait_for_gateway; then + exec 3>&- + openclaw_e2e_stop_process "$wizard_pid" + rm -rf "$input_fifo_dir" + stop_gateway "$gw_pid" + exit 1 + fi fi - "$send_fn" - - if ! wait "$wizard_pid"; then - wizard_status=$? + "$send_fn" || wizard_status=$? + if [ "$wizard_status" -ne 0 ]; then exec 3>&- - rm -f "$input_fifo" + openclaw_e2e_stop_process "$wizard_pid" + rm -rf "$input_fifo_dir" + stop_gateway "$gw_pid" + echo "Wizard input driver exited with status $wizard_status" + if [ -f "$log_path" ]; then + tail -n 160 "$log_path" || true + fi + exit "$wizard_status" + fi + + wait "$wizard_pid" || wizard_status=$? + if [ "$wizard_status" -ne 0 ]; then + exec 3>&- + rm -rf "$input_fifo_dir" stop_gateway "$gw_pid" echo "Wizard exited with status $wizard_status" if [ -f "$log_path" ]; then @@ -115,7 +144,7 @@ run_wizard_cmd() { exit "$wizard_status" fi exec 3>&- - rm -f "$input_fifo" + rm -rf "$input_fifo_dir" stop_gateway "$gw_pid" if [ -n "$validate_fn" ]; then "$validate_fn" "$log_path" diff --git a/scripts/e2e/lib/release-typed-onboarding/scenario.sh b/scripts/e2e/lib/release-typed-onboarding/scenario.sh index d0510dfa5a7c..cdf51be529db 100755 --- a/scripts/e2e/lib/release-typed-onboarding/scenario.sh +++ b/scripts/e2e/lib/release-typed-onboarding/scenario.sh @@ -23,8 +23,15 @@ MOCK_REQUEST_LOG="/tmp/openclaw-release-typed-onboarding-openai.jsonl" export SUCCESS_MARKER MOCK_REQUEST_LOG mock_pid="" +wizard_pid="" +input_fifo_dir="" cleanup() { + exec 3>&- 2>/dev/null || true + openclaw_e2e_stop_process "${wizard_pid:-}" openclaw_e2e_stop_process "${mock_pid:-}" + if [ -n "${input_fifo_dir:-}" ]; then + rm -rf "$input_fifo_dir" + fi } trap cleanup EXIT @@ -80,7 +87,8 @@ entry="$(openclaw_e2e_package_entrypoint "$package_root")" mock_pid="$(openclaw_e2e_start_mock_openai "$MOCK_PORT" /tmp/openclaw-release-typed-onboarding-openai.log)" openclaw_e2e_wait_mock_openai "$MOCK_PORT" -input_fifo="$(mktemp -u "/tmp/openclaw-release-typed-onboarding.XXXXXX")" +input_fifo_dir="$(mktemp -d "/tmp/openclaw-release-typed-onboarding.XXXXXX")" +input_fifo="$input_fifo_dir/stdin.fifo" mkfifo "$input_fifo" openclaw_e2e_run_script_with_pty "node \"$entry\" onboard --flow quickstart --mode local --auth-choice skip --gateway-port \"$PORT\" --gateway-bind loopback --skip-daemon --skip-ui --skip-channels --skip-skills --skip-health" /tmp/openclaw-release-typed-onboarding.log <"$input_fifo" >/dev/null 2>&1 & wizard_pid="$!" @@ -95,8 +103,10 @@ send $' \r' 0.4 send $'\r' 0.4 wait "$wizard_pid" +wizard_pid="" exec 3>&- -rm -f "$input_fifo" +rm -rf "$input_fifo_dir" +input_fifo_dir="" openclaw onboard \ --non-interactive \ diff --git a/scripts/e2e/lib/run-with-pty.mjs b/scripts/e2e/lib/run-with-pty.mjs new file mode 100644 index 000000000000..5ebaf3895143 --- /dev/null +++ b/scripts/e2e/lib/run-with-pty.mjs @@ -0,0 +1,49 @@ +#!/usr/bin/env node +import fs from "node:fs"; +import process from "node:process"; +import { spawn } from "@lydell/node-pty"; + +const [logPath, command, ...args] = process.argv.slice(2); + +if (!logPath || !command) { + console.error("usage: run-with-pty.mjs [args...]"); + process.exit(2); +} + +const log = fs.createWriteStream(logPath, { flags: "w" }); +const pty = spawn(command, args, { + name: process.env.TERM || "xterm-256color", + cols: Number(process.env.COLUMNS || 120), + rows: Number(process.env.LINES || 40), + cwd: process.cwd(), + env: process.env, +}); + +let exiting = false; + +pty.onData((data) => { + log.write(data); + process.stdout.write(data); +}); + +pty.onExit(({ exitCode, signal }) => { + exiting = true; + log.end(() => { + if (typeof exitCode === "number") { + process.exit(exitCode); + } + process.exit(signal ? 128 + signal : 1); + }); +}); + +process.stdin.on("data", (chunk) => { + pty.write(chunk.toString("utf8")); +}); + +for (const signal of ["SIGINT", "SIGTERM"]) { + process.on(signal, () => { + if (!exiting) { + pty.kill(signal); + } + }); +} diff --git a/scripts/lib/openclaw-e2e-instance.sh b/scripts/lib/openclaw-e2e-instance.sh index 2f3efad7f5a6..098884ad1d71 100644 --- a/scripts/lib/openclaw-e2e-instance.sh +++ b/scripts/lib/openclaw-e2e-instance.sh @@ -111,6 +111,8 @@ openclaw_e2e_run_script_with_pty() { local log_path="$2" if script --version >/dev/null 2>&1; then script -q -f -c "$command" "$log_path" + elif node -e 'import("@lydell/node-pty")' >/dev/null 2>&1; then + node scripts/e2e/lib/run-with-pty.mjs "$log_path" /bin/bash -lc "$command" else script -q -F "$log_path" /bin/bash -lc "$command" fi diff --git a/scripts/lib/openclaw-test-state.mjs b/scripts/lib/openclaw-test-state.mjs index a8986b7bda18..c39e374475eb 100644 --- a/scripts/lib/openclaw-test-state.mjs +++ b/scripts/lib/openclaw-test-state.mjs @@ -345,6 +345,8 @@ export function renderShellSnippet(options = {}) { const homeTemplate = `openclaw-${label}-${scenario}-home.XXXXXX`; const lines = [ 'OPENCLAW_TEST_STATE_TMP_ROOT="${OPENCLAW_TEST_STATE_TMPDIR:-${TMPDIR:-/tmp}}"', + 'OPENCLAW_TEST_STATE_TMP_ROOT="${OPENCLAW_TEST_STATE_TMP_ROOT%/}"', + '[ -n "$OPENCLAW_TEST_STATE_TMP_ROOT" ] || OPENCLAW_TEST_STATE_TMP_ROOT="/tmp"', "export OPENCLAW_TEST_STATE_TMP_ROOT", 'mkdir -p "$OPENCLAW_TEST_STATE_TMP_ROOT"', `OPENCLAW_TEST_STATE_HOME="$(mktemp -d "$OPENCLAW_TEST_STATE_TMP_ROOT/${homeTemplate}")"`, @@ -388,6 +390,8 @@ export function renderShellFunction() { label="$(printf "%s" "$label" | tr -cs "A-Za-z0-9_.-" "-" | sed -e "s/^-*//" -e "s/-*$//")" [ -n "$label" ] || label="state" local tmp_root="\${OPENCLAW_TEST_STATE_TMPDIR:-\${TMPDIR:-/tmp}}" + tmp_root="\${tmp_root%/}" + [ -n "$tmp_root" ] || tmp_root="/tmp" mkdir -p "$tmp_root" OPENCLAW_TEST_STATE_HOME="$(mktemp -d "$tmp_root/openclaw-$label-$scenario-home.XXXXXX")" ;; diff --git a/test/scripts/e2e-run-with-pty.test.ts b/test/scripts/e2e-run-with-pty.test.ts new file mode 100644 index 000000000000..7b7326f72e16 --- /dev/null +++ b/test/scripts/e2e-run-with-pty.test.ts @@ -0,0 +1,68 @@ +import { spawn } from "node:child_process"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); +const scriptPath = path.join(repoRoot, "scripts/e2e/lib/run-with-pty.mjs"); + +function runPtyProbe(logPath: string): Promise<{ code: number | null; stdout: string; stderr: string }> { + return new Promise((resolve, reject) => { + const child = spawn( + process.execPath, + [ + scriptPath, + logPath, + "/bin/bash", + "-lc", + 'printf "prompt\\n"; IFS= read -r value; printf "got:%s\\n" "$value"', + ], + { stdio: ["pipe", "pipe", "pipe"] }, + ); + let stdout = ""; + let stderr = ""; + const timeout = setTimeout(() => { + child.kill("SIGTERM"); + reject(new Error("PTY probe timed out")); + }, 10_000); + + child.stdout.setEncoding("utf8"); + child.stderr.setEncoding("utf8"); + child.stdout.on("data", (chunk) => { + stdout += chunk; + }); + child.stderr.on("data", (chunk) => { + stderr += chunk; + }); + child.on("error", (error) => { + clearTimeout(timeout); + reject(error); + }); + child.on("close", (code) => { + clearTimeout(timeout); + resolve({ code, stdout, stderr }); + }); + child.stdin.end("abc\n"); + }); +} + +describe("run-with-pty", () => { + it("forwards stdin through a PTY and writes the transcript log", async () => { + const tempRoot = await mkdtemp(path.join(os.tmpdir(), "openclaw-run-with-pty-")); + const logPath = path.join(tempRoot, "pty.log"); + try { + const result = await runPtyProbe(logPath); + const log = await readFile(logPath, "utf8"); + + expect(result).toMatchObject({ code: 0, stderr: "" }); + expect(result.stdout).toContain("prompt"); + expect(result.stdout).toContain("got:abc"); + expect(log).toContain("prompt"); + expect(log).toContain("got:abc"); + } finally { + await rm(tempRoot, { recursive: true, force: true }); + } + }); +}); diff --git a/test/scripts/e2e-shell-tempfiles.test.ts b/test/scripts/e2e-shell-tempfiles.test.ts new file mode 100644 index 000000000000..d5d62cd103f4 --- /dev/null +++ b/test/scripts/e2e-shell-tempfiles.test.ts @@ -0,0 +1,41 @@ +import { readdir, readFile } from "node:fs/promises"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +async function listShellScripts(dir: string): Promise { + const entries = await readdir(dir, { withFileTypes: true }); + const scripts: string[] = []; + + for (const entry of entries.toSorted((a, b) => a.name.localeCompare(b.name))) { + const entryPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + scripts.push(...(await listShellScripts(entryPath))); + } else if (entry.isFile() && entry.name.endsWith(".sh")) { + scripts.push(entryPath); + } + } + + return scripts; +} + +describe("e2e shell tempfile hygiene", () => { + it("does not allocate FIFO paths with mktemp -u", async () => { + const offenders: string[] = []; + + for (const scriptPath of await listShellScripts("scripts/e2e")) { + const contents = await readFile(path.resolve(scriptPath), "utf8"); + if (contents.includes("mktemp -u")) { + offenders.push(scriptPath); + } + } + + expect(offenders).toEqual([]); + }); + + it("preserves wizard exit status when reporting failures", async () => { + const contents = await readFile("scripts/e2e/lib/onboard/scenario.sh", "utf8"); + + expect(contents).not.toContain('if ! wait "$wizard_pid"'); + expect(contents).toContain('wait "$wizard_pid" || wizard_status=$?'); + }); +}); diff --git a/test/scripts/openclaw-test-state.test.ts b/test/scripts/openclaw-test-state.test.ts index 3c3fc9285f1d..abcb5a2d54de 100644 --- a/test/scripts/openclaw-test-state.test.ts +++ b/test/scripts/openclaw-test-state.test.ts @@ -137,6 +137,19 @@ describe("scripts/lib/openclaw-test-state", () => { `^${escapeRegex(customTemp)}/openclaw-update-channel-switch-update-stable-home\\.`, ), ); + + const trailingSlashProbe = await execFileAsync("bash", [ + "-lc", + `export OPENCLAW_TEST_STATE_TMPDIR=${shellQuote(`${customTemp}/`)}; source ${shellQuote(snippetFile)}; node -e 'process.stdout.write(JSON.stringify({home:process.env.HOME,tmpRoot:process.env.OPENCLAW_TEST_STATE_TMP_ROOT,stateDir:process.env.OPENCLAW_STATE_DIR}));'; rm -rf "$HOME"`, + ]); + const trailingSlashPayload = JSON.parse(trailingSlashProbe.stdout); + expect(trailingSlashPayload.tmpRoot).toBe(customTemp); + expect(trailingSlashPayload.home).toMatch( + new RegExp( + `^${escapeRegex(customTemp)}/openclaw-update-channel-switch-update-stable-home\\.`, + ), + ); + expect(trailingSlashPayload.stateDir).toBe(`${trailingSlashPayload.home}/.openclaw`); } finally { await fs.rm(tempRoot, { recursive: true, force: true }); } @@ -246,6 +259,17 @@ describe("scripts/lib/openclaw-test-state", () => { expect(payload.secretKey).toMatch(secretKeyPattern); expect(payload.config).toStrictEqual({}); + const trailingTmpDir = path.join(tempRoot, "function-trailing-tmp"); + const trailingProbe = await execFileAsync("bash", [ + "-lc", + `export OPENCLAW_TEST_STATE_TMPDIR=${shellQuote(`${trailingTmpDir}/`)}; source ${shellQuote(snippetFile)}; openclaw_test_state_create "onboard case" minimal; node -e 'process.stdout.write(JSON.stringify({home:process.env.HOME,tmpDir:process.env.OPENCLAW_TEST_STATE_TMPDIR,stateDir:process.env.OPENCLAW_STATE_DIR,workspace:process.env.OPENCLAW_TEST_WORKSPACE_DIR}));'; rm -rf "$HOME"`, + ]); + + const trailingPayload = JSON.parse(trailingProbe.stdout); + expect(trailingPayload.home).toBe(`${trailingTmpDir}/${path.basename(trailingPayload.home)}`); + expect(trailingPayload.stateDir).toBe(`${trailingPayload.home}/.openclaw`); + expect(trailingPayload.workspace).toBe(`${trailingPayload.home}/workspace`); + const existingHome = path.join(tempRoot, "existing-home"); const existingProbe = await execFileAsync("bash", [ "-lc",