From 2fe7b5e8c98e5dce063aff29e5bab03d9c7c6259 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 7 Jun 2026 08:42:13 +0200 Subject: [PATCH] fix(dev): harden smoke log diagnostics --- scripts/dev/ios-pull-gateway-log.sh | 5 ++ scripts/dev/tui-pty-test-watch.ts | 78 ++++++++++++++++------ test/scripts/dev-tooling-safety.test.ts | 62 +++++++++++++++++- test/scripts/ios-pull-gateway-log.test.ts | 80 +++++++++++++++++++---- 4 files changed, 193 insertions(+), 32 deletions(-) diff --git a/scripts/dev/ios-pull-gateway-log.sh b/scripts/dev/ios-pull-gateway-log.sh index 3d80f0201b34..a5a6139642b7 100755 --- a/scripts/dev/ios-pull-gateway-log.sh +++ b/scripts/dev/ios-pull-gateway-log.sh @@ -27,5 +27,10 @@ xcrun devicectl device copy from \ --source Documents/openclaw-gateway.log \ --destination "$DEST" >/dev/null +if [[ ! -s "$DEST" ]]; then + echo "Gateway log pull produced an empty file: $DEST" >&2 + exit 1 +fi + echo "Pulled to: $DEST" tail -n 200 "$DEST" diff --git a/scripts/dev/tui-pty-test-watch.ts b/scripts/dev/tui-pty-test-watch.ts index cd21c99d1877..a604897b85e7 100644 --- a/scripts/dev/tui-pty-test-watch.ts +++ b/scripts/dev/tui-pty-test-watch.ts @@ -1,6 +1,6 @@ // Tui Pty Test Watch script supports OpenClaw repository automation. import { spawn } from "node:child_process"; -import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { mkdir, open, writeFile } from "node:fs/promises"; import { createRequire } from "node:module"; import path from "node:path"; import { pathToFileURL } from "node:url"; @@ -24,6 +24,8 @@ const DEFAULT_PTY_COLS = 100; const DEFAULT_PTY_ROWS = 30; const CHILD_SIGTERM_GRACE_MS = 500; const CHILD_SIGKILL_GRACE_MS = 5_000; +const MIRROR_READ_CHUNK_BYTES = 1024 * 1024; +const CHILD_OUTPUT_TAIL_BYTES = 128 * 1024; type KillableChild = { pid?: number; @@ -155,16 +157,54 @@ async function createMirrorFile(mirrorPath: string): Promise { await writeFile(mirrorPath, "", "utf8"); } -async function readNewMirrorData(mirrorPath: string, offset: number) { - const data = await readFile(mirrorPath); - const nextOffset = data.byteLength; - if (nextOffset < offset) { - return { chunk: data, offset: nextOffset }; +async function readNewMirrorData( + mirrorPath: string, + offset: number, + maxChunkBytes = MIRROR_READ_CHUNK_BYTES, +) { + const file = await open(mirrorPath, "r"); + try { + const stats = await file.stat(); + const readOffset = stats.size < offset ? 0 : offset; + const availableBytes = stats.size - readOffset; + if (availableBytes <= 0) { + return { chunk: Buffer.alloc(0), offset: readOffset }; + } + const bytesToRead = Math.min(availableBytes, maxChunkBytes); + const buffer = Buffer.alloc(bytesToRead); + const { bytesRead } = await file.read(buffer, 0, bytesToRead, readOffset); + return { chunk: buffer.subarray(0, bytesRead), offset: readOffset + bytesRead }; + } finally { + await file.close(); } - if (nextOffset === offset) { - return { chunk: Buffer.alloc(0), offset }; +} + +function appendBufferTail(current: Buffer, chunk: Buffer, maxBytes = CHILD_OUTPUT_TAIL_BYTES) { + if (chunk.byteLength >= maxBytes) { + return chunk.subarray(chunk.byteLength - maxBytes); + } + if (current.byteLength + chunk.byteLength <= maxBytes) { + return current.byteLength === 0 ? Buffer.from(chunk) : Buffer.concat([current, chunk]); + } + const keepBytes = maxBytes - chunk.byteLength; + return Buffer.concat([current.subarray(current.byteLength - keepBytes), chunk]); +} + +async function drainNewMirrorData( + mirrorPath: string, + offset: number, + onChunk: (chunk: Buffer) => void, + maxChunkBytes = MIRROR_READ_CHUNK_BYTES, +) { + let nextOffset = offset; + for (;;) { + const result = await readNewMirrorData(mirrorPath, nextOffset, maxChunkBytes); + nextOffset = result.offset; + if (result.chunk.byteLength === 0) { + return nextOffset; + } + onChunk(result.chunk); } - return { chunk: data.subarray(offset), offset: nextOffset }; } async function main(): Promise { @@ -200,8 +240,8 @@ async function main(): Promise { }, ); - let childStdout = ""; - let childStderr = ""; + let childStdout = Buffer.alloc(0); + let childStderr = Buffer.alloc(0); let restored = false; let mirrorOffset = 0; let mirrorFilterPending = ""; @@ -309,10 +349,10 @@ async function main(): Promise { } child.stdout?.on("data", (chunk: Buffer) => { - childStdout += chunk.toString("utf8"); + childStdout = appendBufferTail(childStdout, chunk); }); child.stderr?.on("data", (chunk: Buffer) => { - childStderr += chunk.toString("utf8"); + childStderr = appendBufferTail(childStderr, chunk); }); type ChildExit = { code: number | null; signal: NodeJS.Signals | null }; @@ -345,10 +385,7 @@ async function main(): Promise { await delay(sawMirrorOutput ? 25 : 250); } - const result = await readNewMirrorData(options.mirrorPath, mirrorOffset); - if (result.chunk.byteLength > 0) { - writeMirrorChunk(result.chunk); - } + mirrorOffset = await drainNewMirrorData(options.mirrorPath, mirrorOffset, writeMirrorChunk); } finally { if (!childExit) { stopChild(); @@ -368,10 +405,10 @@ async function main(): Promise { childExit = await childFinished; } - if (childStdout) { + if (childStdout.byteLength > 0) { process.stdout.write(childStdout); } - if (childStderr) { + if (childStderr.byteLength > 0) { process.stderr.write(childStderr); } @@ -393,6 +430,9 @@ if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) { } export const testing = { + appendBufferTail, createChildStopper, + drainNewMirrorData, + readNewMirrorData, signalChildProcessTree, }; diff --git a/test/scripts/dev-tooling-safety.test.ts b/test/scripts/dev-tooling-safety.test.ts index ba9b503976d2..75d7d71ff953 100644 --- a/test/scripts/dev-tooling-safety.test.ts +++ b/test/scripts/dev-tooling-safety.test.ts @@ -19,8 +19,13 @@ import { redactJsonValueForDevToolLog, } from "../../scripts/lib/dev-tooling-safety.ts"; -afterEach(() => { +const tempDirs: string[] = []; + +afterEach(async () => { vi.useRealTimers(); + for (const dir of tempDirs.splice(0)) { + await fs.rm(dir, { force: true, recursive: true }); + } }); describe("dev tooling safety helpers", () => { @@ -224,6 +229,61 @@ describe("script-specific dev tooling hardening", () => { expect(signals).toEqual(["SIGINT", "SIGTERM", "SIGKILL"]); }); + it("reads TUI PTY mirror updates incrementally with a bounded chunk", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-tui-watch-test-")); + tempDirs.push(tempRoot); + const mirrorPath = path.join(tempRoot, "mirror.ansi"); + await fs.writeFile(mirrorPath, "first-second-third", "utf8"); + + const first = await tuiPtyWatchTesting.readNewMirrorData(mirrorPath, 0, 6); + expect(first.chunk.toString("utf8")).toBe("first-"); + expect(first.offset).toBe(6); + + const second = await tuiPtyWatchTesting.readNewMirrorData(mirrorPath, first.offset, 6); + expect(second.chunk.toString("utf8")).toBe("second"); + expect(second.offset).toBe(12); + }); + + it("restarts TUI PTY mirror reads when the mirror file is truncated", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-tui-watch-test-")); + tempDirs.push(tempRoot); + const mirrorPath = path.join(tempRoot, "mirror.ansi"); + await fs.writeFile(mirrorPath, "fresh", "utf8"); + + const result = await tuiPtyWatchTesting.readNewMirrorData(mirrorPath, 10, 1024); + + expect(result.chunk.toString("utf8")).toBe("fresh"); + expect(result.offset).toBe(5); + }); + + it("drains all pending TUI PTY mirror chunks after the child exits", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-tui-watch-test-")); + tempDirs.push(tempRoot); + const mirrorPath = path.join(tempRoot, "mirror.ansi"); + await fs.writeFile(mirrorPath, "first-second-third", "utf8"); + const chunks: string[] = []; + + const offset = await tuiPtyWatchTesting.drainNewMirrorData( + mirrorPath, + 0, + (chunk: Buffer) => chunks.push(chunk.toString("utf8")), + 6, + ); + + expect(chunks).toEqual(["first-", "second", "-third"]); + expect(offset).toBe("first-second-third".length); + }); + + it("keeps only diagnostic tails from noisy TUI PTY child output", () => { + const retained = tuiPtyWatchTesting.appendBufferTail( + Buffer.from("0123456789", "utf8"), + Buffer.from("abcdef", "utf8"), + 8, + ); + + expect(retained.toString("utf8")).toBe("89abcdef"); + }); + it.runIf(process.platform !== "win32")( "signals the TUI PTY watch process group before falling back to the child", () => { diff --git a/test/scripts/ios-pull-gateway-log.test.ts b/test/scripts/ios-pull-gateway-log.test.ts index 29dac9b9f55b..b8f6c34c6d7b 100644 --- a/test/scripts/ios-pull-gateway-log.test.ts +++ b/test/scripts/ios-pull-gateway-log.test.ts @@ -1,20 +1,76 @@ // Ios Pull Gateway Log tests cover ios pull gateway log script behavior. -import { readFileSync } from "node:fs"; -import { describe, expect, it } from "vitest"; +import { spawnSync } from "node:child_process"; +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; const scriptPath = "scripts/dev/ios-pull-gateway-log.sh"; +const tempDirs: string[] = []; + +function makeTempDir(): string { + const root = mkdtempSync(path.join(tmpdir(), "openclaw-ios-log-pull-")); + tempDirs.push(root); + return root; +} + +function runWithFakeXcrun( + root: string, + fakeXcrunBody: string, + destPath: string, +): ReturnType { + const binDir = path.join(root, "bin"); + const xcrunPath = path.join(binDir, "xcrun"); + mkdirSync(binDir); + writeFileSync( + xcrunPath, + ["#!/usr/bin/env bash", "set -euo pipefail", fakeXcrunBody, ""].join("\n"), + ); + chmodSync(xcrunPath, 0o755); + + return spawnSync("bash", [scriptPath, "device-udid", "ai.openclaw.ios.dev", destPath], { + cwd: process.cwd(), + encoding: "utf8", + env: { + ...process.env, + PATH: `${binDir}${path.delimiter}${process.env.PATH ?? ""}`, + }, + }); +} + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { force: true, recursive: true }); + } +}); describe("scripts/dev/ios-pull-gateway-log.sh", () => { - it("does not bake local device or bundle identifiers into the log pull helper", () => { - const script = readFileSync(scriptPath, "utf8"); + it("fails when the copied gateway log is empty", () => { + const root = makeTempDir(); + const destPath = path.join(root, "openclaw-gateway.log"); + const result = runWithFakeXcrun( + root, + 'while [[ "$#" -gt 0 ]]; do if [[ "$1" == "--destination" ]]; then shift; : > "$1"; fi; shift || break; done', + destPath, + ); - expect(script).toContain('DEVICE_UDID="${1:-${OPENCLAW_IOS_DEVICE_UDID:-}}"'); - expect(script).toContain('BUNDLE_ID="${2:-${OPENCLAW_IOS_BUNDLE_ID:-}}"'); - expect(script).toContain('DEST="${3:-${OPENCLAW_IOS_GATEWAY_LOG_DEST:-}}"'); - expect(script).toContain('mktemp -d "${TMPDIR:-/tmp}/openclaw-ios-gateway.XXXXXX"'); - expect(script).toContain("exit 2"); - expect(script).not.toMatch(/DEVICE_UDID="\$\{1:-[0-9A-F-]+/u); - expect(script).not.toMatch(/BUNDLE_ID="\$\{2:-ai\.openclaw\.ios\.dev\.[^}]+/u); - expect(script).not.toContain("/tmp/openclaw-gateway.log"); + expect(result.status).toBe(1); + expect(result.stderr).toContain("Gateway log pull produced an empty file"); + expect(result.stdout).not.toContain("Pulled to:"); + }); + + it("prints the pulled gateway log tail when the copied file has content", () => { + const root = makeTempDir(); + const destPath = path.join(root, "openclaw-gateway.log"); + const result = runWithFakeXcrun( + root, + 'while [[ "$#" -gt 0 ]]; do if [[ "$1" == "--destination" ]]; then shift; printf "gateway ready\\n" > "$1"; fi; shift || break; done', + destPath, + ); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain(`Pulled to: ${destPath}`); + expect(result.stdout).toContain("gateway ready"); }); });