From 8f78932059176f3d89edb3607283d3094d27e7c7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 23:16:19 +0100 Subject: [PATCH] test: harden QA cleanup and update preflight --- extensions/codex/harness.ts | 5 ++- .../codex/src/app-server/client.test.ts | 32 ++++++++++++++ extensions/codex/src/app-server/client.ts | 30 ++++++++++--- .../codex/src/app-server/shared-client.ts | 12 +++++ extensions/codex/src/app-server/transport.ts | 44 +++++++++++++++++++ extensions/qa-lab/src/lab-server.test.ts | 2 + extensions/qa-lab/src/lab-server.ts | 1 + scripts/e2e/parallels-npm-update-smoke.sh | 16 ++++++- 8 files changed, 132 insertions(+), 10 deletions(-) diff --git a/extensions/codex/harness.ts b/extensions/codex/harness.ts index 8ad81e283fa..c1074979166 100644 --- a/extensions/codex/harness.ts +++ b/extensions/codex/harness.ts @@ -48,8 +48,9 @@ export function createCodexAppServerAgentHarness(options?: { } }, dispose: async () => { - const { clearSharedCodexAppServerClient } = await import("./src/app-server/shared-client.js"); - clearSharedCodexAppServerClient(); + const { clearSharedCodexAppServerClientAndWait } = + await import("./src/app-server/shared-client.js"); + await clearSharedCodexAppServerClientAndWait(); }, }; } diff --git a/extensions/codex/src/app-server/client.test.ts b/extensions/codex/src/app-server/client.test.ts index ad4c22f7565..a8de9cafe11 100644 --- a/extensions/codex/src/app-server/client.test.ts +++ b/extensions/codex/src/app-server/client.test.ts @@ -231,6 +231,38 @@ describe("CodexAppServerClient", () => { expect(process.kill).toHaveBeenCalledWith("SIGKILL"); expect(process.unref).toHaveBeenCalledTimes(1); }); + + it("waits for app-server transport exit during async shutdown", async () => { + vi.useFakeTimers(); + const process = Object.assign(new EventEmitter(), { + stdin: { + write: vi.fn(), + end: vi.fn(), + destroy: vi.fn(), + unref: vi.fn(), + }, + stdout: Object.assign(new PassThrough(), { unref: vi.fn() }), + stderr: Object.assign(new PassThrough(), { unref: vi.fn() }), + exitCode: null as number | null, + signalCode: null as string | null, + kill: vi.fn(), + unref: vi.fn(), + }); + + const closed = __testing.closeCodexAppServerTransportAndWait(process, { + exitTimeoutMs: 100, + forceKillDelayMs: 25, + }); + await vi.advanceTimersByTimeAsync(25); + + expect(process.kill).toHaveBeenCalledWith("SIGTERM"); + expect(process.kill).toHaveBeenCalledWith("SIGKILL"); + process.signalCode = "SIGKILL"; + process.emit("exit"); + + await expect(closed).resolves.toBe(true); + }); + it("handles stdin write errors without crashing the process", async () => { const harness = createClientHarness(); clients.push(harness.client); diff --git a/extensions/codex/src/app-server/client.ts b/extensions/codex/src/app-server/client.ts index 0e6191956d7..de58d3aad62 100644 --- a/extensions/codex/src/app-server/client.ts +++ b/extensions/codex/src/app-server/client.ts @@ -16,7 +16,11 @@ import { } from "./protocol.js"; import { createStdioTransport } from "./transport-stdio.js"; import { createWebSocketTransport } from "./transport-websocket.js"; -import { closeCodexAppServerTransport, type CodexAppServerTransport } from "./transport.js"; +import { + closeCodexAppServerTransport, + closeCodexAppServerTransportAndWait, + type CodexAppServerTransport, +} from "./transport.js"; export const MIN_CODEX_APP_SERVER_VERSION = "0.125.0"; const CODEX_APP_SERVER_PARSE_LOG_MAX = 500; @@ -225,15 +229,20 @@ export class CodexAppServerClient { } close(): void { - if (this.closed) { + if (!this.markClosed(new Error("codex app-server client is closed"))) { return; } - this.closed = true; - this.lines.close(); - this.rejectPendingRequests(new Error("codex app-server client is closed")); closeCodexAppServerTransport(this.child); } + async closeAndWait(options?: { + exitTimeoutMs?: number; + forceKillDelayMs?: number; + }): Promise { + this.markClosed(new Error("codex app-server client is closed")); + await closeCodexAppServerTransportAndWait(this.child, options); + } + private writeMessage(message: RpcRequest | RpcResponse): void { if (this.closed) { return; @@ -325,13 +334,19 @@ export class CodexAppServerClient { } private closeWithError(error: Error): void { + if (this.markClosed(error)) { + closeCodexAppServerTransport(this.child); + } + } + + private markClosed(error: Error): boolean { if (this.closed) { - return; + return false; } this.closed = true; this.lines.close(); this.rejectPendingRequests(error); - closeCodexAppServerTransport(this.child); + return true; } private rejectPendingRequests(error: Error): void { @@ -485,5 +500,6 @@ function formatExitValue(value: unknown): string { export const __testing = { closeCodexAppServerTransport, + closeCodexAppServerTransportAndWait, redactCodexAppServerLinePreview, } as const; diff --git a/extensions/codex/src/app-server/shared-client.ts b/extensions/codex/src/app-server/shared-client.ts index 1338e52d92c..407e0bc4309 100644 --- a/extensions/codex/src/app-server/shared-client.ts +++ b/extensions/codex/src/app-server/shared-client.ts @@ -120,6 +120,18 @@ export function clearSharedCodexAppServerClient(): void { client?.close(); } +export async function clearSharedCodexAppServerClientAndWait(options?: { + exitTimeoutMs?: number; + forceKillDelayMs?: number; +}): Promise { + const state = getSharedCodexAppServerClientState(); + const client = state.client; + state.client = undefined; + state.promise = undefined; + state.key = undefined; + await client?.closeAndWait(options); +} + function clearSharedClientIfCurrent(client: CodexAppServerClient): void { const state = getSharedCodexAppServerClientState(); if (state.client !== client) { diff --git a/extensions/codex/src/app-server/transport.ts b/extensions/codex/src/app-server/transport.ts index b11fd764a55..2e7d41e7509 100644 --- a/extensions/codex/src/app-server/transport.ts +++ b/extensions/codex/src/app-server/transport.ts @@ -21,6 +21,7 @@ export type CodexAppServerTransport = { kill?: (signal?: NodeJS.Signals) => unknown; unref?: () => unknown; once: (event: string, listener: (...args: unknown[]) => void) => unknown; + off?: (event: string, listener: (...args: unknown[]) => void) => unknown; }; export function closeCodexAppServerTransport( @@ -50,12 +51,55 @@ export function closeCodexAppServerTransport( child.stdin.unref?.(); } +export async function closeCodexAppServerTransportAndWait( + child: CodexAppServerTransport, + options: { exitTimeoutMs?: number; forceKillDelayMs?: number } = {}, +): Promise { + if (!hasCodexAppServerTransportExited(child)) { + closeCodexAppServerTransport(child, options); + } + return await waitForCodexAppServerTransportExit(child, options.exitTimeoutMs ?? 2_000); +} + function hasCodexAppServerTransportExited(child: CodexAppServerTransport): boolean { return child.exitCode !== null && child.exitCode !== undefined ? true : child.signalCode !== null && child.signalCode !== undefined; } +async function waitForCodexAppServerTransportExit( + child: CodexAppServerTransport, + timeoutMs: number, +): Promise { + if (hasCodexAppServerTransportExited(child)) { + return true; + } + return await new Promise((resolve) => { + let settled = false; + const onExit = () => { + if (settled) { + return; + } + settled = true; + clearTimeout(timeout); + resolve(true); + }; + const timeout = setTimeout( + () => { + if (settled) { + return; + } + settled = true; + child.off?.("exit", onExit); + resolve(false); + }, + Math.max(1, timeoutMs), + ); + timeout.unref?.(); + child.once("exit", onExit); + }); +} + function signalCodexAppServerTransport( child: CodexAppServerTransport, signal: NodeJS.Signals, diff --git a/extensions/qa-lab/src/lab-server.test.ts b/extensions/qa-lab/src/lab-server.test.ts index ca6fa268c13..9160df70eea 100644 --- a/extensions/qa-lab/src/lab-server.test.ts +++ b/extensions/qa-lab/src/lab-server.test.ts @@ -85,6 +85,7 @@ const captureMock = vi.hoisted(() => { readBlob() { return null; }, + close: vi.fn(), deleteSessions(sessionIds: string[]) { const ids = new Set(sessionIds); for (let index = sessions.length - 1; index >= 0; index -= 1) { @@ -106,6 +107,7 @@ const captureMock = vi.hoisted(() => { reset() { sessions.splice(0); events.splice(0); + store.close.mockClear(); }, }; }); diff --git a/extensions/qa-lab/src/lab-server.ts b/extensions/qa-lab/src/lab-server.ts index 26fbbaa2bab..11d544a238f 100644 --- a/extensions/qa-lab/src/lab-server.ts +++ b/extensions/qa-lab/src/lab-server.ts @@ -639,6 +639,7 @@ export async function startQaLabServer( await runnerModelCatalogPromise?.catch(() => undefined); await gateway?.stop(); await closeQaHttpServer(server); + captureStore.close(); }, }; labHandle = lab; diff --git a/scripts/e2e/parallels-npm-update-smoke.sh b/scripts/e2e/parallels-npm-update-smoke.sh index efbafd46b00..64566b87d4f 100755 --- a/scripts/e2e/parallels-npm-update-smoke.sh +++ b/scripts/e2e/parallels-npm-update-smoke.sh @@ -443,7 +443,7 @@ resolve_registry_target_version() { if [[ "$spec" != openclaw@* ]]; then spec="openclaw@$spec" fi - npm view "$spec" version 2>/dev/null || true + npm view "$spec" version 2>/dev/null | tail -n 1 | tr -d '\r' || true } is_explicit_package_target() { @@ -451,6 +451,19 @@ is_explicit_package_target() { [[ "$target" == *"://"* || "$target" == *"#"* || "$target" =~ ^(file|github|git\+ssh|git\+https|git\+http|git\+file|npm): ]] } +preflight_registry_update_target() { + local baseline_version target_version + [[ -n "$UPDATE_TARGET" && "$UPDATE_TARGET" != "local-main" ]] || return 0 + is_explicit_package_target "$UPDATE_TARGET" && return 0 + + baseline_version="$(resolve_registry_target_version "$PACKAGE_SPEC")" + target_version="$(resolve_registry_target_version "$UPDATE_TARGET")" + [[ -n "$baseline_version" && -n "$target_version" ]] || return 0 + if [[ "$baseline_version" == "$target_version" ]]; then + die "--update-target $UPDATE_TARGET resolves to openclaw@$target_version, same as baseline $PACKAGE_SPEC; publish or choose a newer --update-target before running VM update coverage" + fi +} + write_windows_update_script() { WINDOWS_UPDATE_SCRIPT_PATH="$MAIN_TGZ_DIR/openclaw-main-update.ps1" cat >"$WINDOWS_UPDATE_SCRIPT_PATH" <<'EOF' @@ -1879,6 +1892,7 @@ LATEST_VERSION="$(resolve_latest_version)" if [[ -z "$PACKAGE_SPEC" ]]; then PACKAGE_SPEC="openclaw@$LATEST_VERSION" fi +preflight_registry_update_target resolve_current_head if platform_enabled linux; then