mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 11:55:47 -06:00
fix(update): fence TUI startup during mutation
This commit is contained in:
@@ -3,6 +3,7 @@ import {
|
||||
listLocalTuiProcesses,
|
||||
quiesceLocalTuiProcessesBeforeUpdate,
|
||||
terminateLocalTuiProcesses,
|
||||
waitForLocalTuiUpdate,
|
||||
} from "./local-tui-processes.js";
|
||||
|
||||
describe("local TUI processes", () => {
|
||||
@@ -74,11 +75,24 @@ describe("local TUI processes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("skips process probing on Windows", () => {
|
||||
const spawnSync = vi.fn();
|
||||
it("lists verified TUI processes on Windows", () => {
|
||||
const spawnSync = vi.fn().mockReturnValue({
|
||||
status: 0,
|
||||
stdout: JSON.stringify([
|
||||
{ ProcessId: 101, CommandLine: "C:\\openclaw.exe tui" },
|
||||
{ ProcessId: 102, CommandLine: "C:\\openclaw.exe gateway" },
|
||||
]),
|
||||
});
|
||||
|
||||
expect(listLocalTuiProcesses({ platform: "win32", spawnSync })).toEqual([]);
|
||||
expect(spawnSync).not.toHaveBeenCalled();
|
||||
expect(
|
||||
listLocalTuiProcesses({
|
||||
platform: "win32",
|
||||
currentPid: 999,
|
||||
spawnSync,
|
||||
readWindowsStartTime: () => 123,
|
||||
}),
|
||||
).toEqual([{ pid: 101, command: "C:\\openclaw.exe tui", startTime: "123" }]);
|
||||
expect(spawnSync).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("terminates stale local TUI processes with a kill fallback", async () => {
|
||||
@@ -162,4 +176,22 @@ describe("local TUI processes", () => {
|
||||
"Update refused: could not stop local TUI clients 101. Close them and retry the update.",
|
||||
);
|
||||
});
|
||||
|
||||
it("holds the startup gate after discovery until the update owner releases it", async () => {
|
||||
const release = vi.fn(async () => {});
|
||||
const lock = await quiesceLocalTuiProcessesBeforeUpdate({
|
||||
list: () => [],
|
||||
acquireLock: vi.fn(async () => ({ lockPath: "test", release })),
|
||||
});
|
||||
|
||||
expect(release).not.toHaveBeenCalled();
|
||||
await lock?.release();
|
||||
expect(release).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("waits for the update gate before TUI startup", async () => {
|
||||
const release = vi.fn(async () => {});
|
||||
await waitForLocalTuiUpdate(vi.fn(async () => ({ lockPath: "test", release })));
|
||||
expect(release).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
import { spawnSync, type SpawnSyncOptionsWithStringEncoding } from "node:child_process";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { sleep } from "../utils/sleep.js";
|
||||
import { getCommandPositionalsWithRootOptions } from "./cli-root-options.js";
|
||||
import { extractErrorCode } from "./errors.js";
|
||||
import { acquireFileLock, type FileLockHandle } from "./file-lock.js";
|
||||
import { getWindowsPowerShellExePath } from "./windows-install-roots.js";
|
||||
import { readWindowsProcessStartTimeSync } from "./windows-port-pids.js";
|
||||
|
||||
export type LocalTuiProcess = {
|
||||
pid: number;
|
||||
@@ -24,13 +28,24 @@ type PsResult = {
|
||||
|
||||
const LOCAL_TUI_SUBCOMMANDS = new Set(["chat", "terminal", "tui"]);
|
||||
const LOCAL_TUI_PROCESS_PROBE_TIMEOUT_MS = 1_000;
|
||||
const LOCAL_TUI_UPDATE_LOCK_PATH = path.join(os.tmpdir(), "openclaw-local-tui-update");
|
||||
const LOCAL_TUI_UPDATE_LOCK_OPTIONS = {
|
||||
stale: 30_000,
|
||||
retries: { retries: 100, factor: 1, minTimeout: 50, maxTimeout: 250 },
|
||||
staleRecovery: "remove-if-unchanged" as const,
|
||||
};
|
||||
|
||||
function tokenizeCommandLine(command: string): string[] {
|
||||
return command.trim().split(/\s+/u).filter(Boolean);
|
||||
}
|
||||
|
||||
function normalizeExecutableName(value: string | undefined): string {
|
||||
return path.basename(value ?? "").replace(/\.exe$/iu, "");
|
||||
return (
|
||||
(value ?? "")
|
||||
.split(/[\\/]/u)
|
||||
.at(-1)
|
||||
?.replace(/\.exe$/iu, "") ?? ""
|
||||
);
|
||||
}
|
||||
|
||||
function isLocalTuiCommand(command: string): boolean {
|
||||
@@ -90,10 +105,42 @@ export function listLocalTuiProcesses(
|
||||
args: string[],
|
||||
options: SpawnSyncOptionsWithStringEncoding,
|
||||
) => PsResult;
|
||||
readWindowsStartTime?: (pid: number) => number | null;
|
||||
} = {},
|
||||
): LocalTuiProcess[] {
|
||||
if ((params.platform ?? process.platform) === "win32") {
|
||||
return [];
|
||||
const result = (params.spawnSync ?? spawnSync)(
|
||||
getWindowsPowerShellExePath(),
|
||||
[
|
||||
"-NoProfile",
|
||||
"-Command",
|
||||
"Get-CimInstance Win32_Process | Select-Object ProcessId,CreationDate,CommandLine | ConvertTo-Json -Compress",
|
||||
],
|
||||
{ encoding: "utf8", killSignal: "SIGKILL", timeout: LOCAL_TUI_PROCESS_PROBE_TIMEOUT_MS },
|
||||
);
|
||||
if (result.error || result.status !== 0 || typeof result.stdout !== "string") {
|
||||
return [];
|
||||
}
|
||||
try {
|
||||
const parsed = JSON.parse(result.stdout) as
|
||||
| { ProcessId?: number; CommandLine?: string }
|
||||
| Array<{ ProcessId?: number; CommandLine?: string }>;
|
||||
return (Array.isArray(parsed) ? parsed : [parsed]).flatMap((entry) => {
|
||||
const pid = entry.ProcessId;
|
||||
const command = entry.CommandLine?.trim();
|
||||
const startTime = pid
|
||||
? (params.readWindowsStartTime ?? readWindowsProcessStartTimeSync)(pid)
|
||||
: null;
|
||||
return pid &&
|
||||
pid !== (params.currentPid ?? process.pid) &&
|
||||
command &&
|
||||
isLocalTuiCommand(command)
|
||||
? [{ pid, command, ...(startTime === null ? {} : { startTime: String(startTime) }) }]
|
||||
: [];
|
||||
});
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
const currentUid = params.currentUid ?? process.getuid?.();
|
||||
if (currentUid === undefined) {
|
||||
@@ -131,6 +178,10 @@ function isProcessAlive(controller: ProcessController, pid: number): boolean {
|
||||
}
|
||||
|
||||
function readProcessStartTime(pid: number): string | undefined {
|
||||
if (process.platform === "win32") {
|
||||
const startTime = readWindowsProcessStartTimeSync(pid);
|
||||
return startTime === null ? undefined : String(startTime);
|
||||
}
|
||||
const result = spawnSync("ps", ["-p", String(pid), "-o", "lstart="], {
|
||||
encoding: "utf8",
|
||||
killSignal: "SIGKILL",
|
||||
@@ -220,19 +271,36 @@ export async function quiesceLocalTuiProcessesBeforeUpdate(
|
||||
overrides: {
|
||||
list?: typeof listLocalTuiProcesses;
|
||||
terminate?: typeof terminateLocalTuiProcesses;
|
||||
acquireLock?: typeof acquireFileLock;
|
||||
} = {},
|
||||
): Promise<void> {
|
||||
): Promise<FileLockHandle | undefined> {
|
||||
if (!overrides.list && (process.env.VITEST || process.env.NODE_ENV === "test")) {
|
||||
return;
|
||||
return undefined;
|
||||
}
|
||||
// Keep startup and discovery in one interprocess order. The updater retains
|
||||
// this gate until mutation ends, so a newly launched TUI cannot enter stale code.
|
||||
const updateLock = await (overrides.acquireLock ?? acquireFileLock)(
|
||||
LOCAL_TUI_UPDATE_LOCK_PATH,
|
||||
LOCAL_TUI_UPDATE_LOCK_OPTIONS,
|
||||
);
|
||||
const processes = (overrides.list ?? listLocalTuiProcesses)();
|
||||
if (processes.length === 0) {
|
||||
return;
|
||||
return updateLock;
|
||||
}
|
||||
const stopped = await (overrides.terminate ?? terminateLocalTuiProcesses)({ processes });
|
||||
if (stopped.failed.length > 0) {
|
||||
await updateLock.release();
|
||||
throw new Error(
|
||||
`Update refused: could not stop local TUI clients ${stopped.failed.join(", ")}. Close them and retry the update.`,
|
||||
);
|
||||
}
|
||||
return updateLock;
|
||||
}
|
||||
|
||||
/** Waits for an in-flight update before a TUI enters its loaded runtime. */
|
||||
export async function waitForLocalTuiUpdate(
|
||||
acquireLock: typeof acquireFileLock = acquireFileLock,
|
||||
): Promise<void> {
|
||||
const lock = await acquireLock(LOCAL_TUI_UPDATE_LOCK_PATH, LOCAL_TUI_UPDATE_LOCK_OPTIONS);
|
||||
await lock.release();
|
||||
}
|
||||
|
||||
@@ -881,12 +881,13 @@ export async function runGlobalPackageUpdateSteps(params: {
|
||||
let stagedInstall: StagedNpmInstall | null = null;
|
||||
let packedInstallDir: string | null = null;
|
||||
let mutationPrepared = false;
|
||||
let tuiUpdateLock: Awaited<ReturnType<typeof quiesceLocalTuiProcessesBeforeUpdate>>;
|
||||
const prepareMutation = async () => {
|
||||
if (mutationPrepared) {
|
||||
return;
|
||||
}
|
||||
await params.beforeMutation?.();
|
||||
await quiesceLocalTuiProcessesBeforeUpdate();
|
||||
tuiUpdateLock = await quiesceLocalTuiProcessesBeforeUpdate();
|
||||
mutationPrepared = true;
|
||||
};
|
||||
|
||||
@@ -1246,6 +1247,7 @@ export async function runGlobalPackageUpdateSteps(params: {
|
||||
failedStep,
|
||||
};
|
||||
} finally {
|
||||
await tuiUpdateLock?.release();
|
||||
await cleanupStagedNpmInstall(stagedInstall);
|
||||
if (packedInstallDir) {
|
||||
await removePathBestEffort(packedInstallDir);
|
||||
|
||||
@@ -50,6 +50,7 @@ export async function prepareGitMutation(params: {
|
||||
}): Promise<{
|
||||
allowGatewayServiceRepair?: boolean;
|
||||
allowGatewayActivation?: boolean;
|
||||
releaseTuiUpdateLock?: () => Promise<void>;
|
||||
}> {
|
||||
const target = await readGitTargetSchemaVersions(params);
|
||||
const preparation = await params.beforeGitMutation?.(
|
||||
@@ -59,8 +60,11 @@ export async function prepareGitMutation(params: {
|
||||
: {}
|
||||
: { metadataUnreadable: target.reason },
|
||||
);
|
||||
await quiesceLocalTuiProcessesBeforeUpdate();
|
||||
return preparation ?? {};
|
||||
const tuiUpdateLock = await quiesceLocalTuiProcessesBeforeUpdate();
|
||||
return {
|
||||
...preparation,
|
||||
...(tuiUpdateLock ? { releaseTuiUpdateLock: tuiUpdateLock.release } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
export async function readBranchName(
|
||||
|
||||
@@ -95,6 +95,12 @@ export async function updateGitCheckout(params: {
|
||||
let allowGatewayServiceRepair = opts.allowGatewayServiceRepair !== false;
|
||||
let allowGatewayActivation = opts.allowGatewayActivation === true;
|
||||
let mutationPrepared = false;
|
||||
let releaseTuiUpdateLock: (() => Promise<void>) | undefined;
|
||||
const releaseTuiUpdateGate = async () => {
|
||||
const release = releaseTuiUpdateLock;
|
||||
releaseTuiUpdateLock = undefined;
|
||||
await release?.();
|
||||
};
|
||||
let createdDevBranchDuringUpdate = false;
|
||||
let devPreflight: Awaited<ReturnType<typeof runGitDevPreflight>> | undefined;
|
||||
let liveBuildStarted = false;
|
||||
@@ -109,6 +115,7 @@ export async function updateGitCheckout(params: {
|
||||
});
|
||||
allowGatewayServiceRepair = preparation.allowGatewayServiceRepair ?? allowGatewayServiceRepair;
|
||||
allowGatewayActivation = preparation.allowGatewayActivation ?? allowGatewayActivation;
|
||||
releaseTuiUpdateLock = preparation.releaseTuiUpdateLock;
|
||||
mutationPrepared = true;
|
||||
};
|
||||
const buildError = (reason: string, status: "error" | "skipped" = "error"): UpdateRunResult => ({
|
||||
@@ -300,6 +307,7 @@ export async function updateGitCheckout(params: {
|
||||
"checkout-failed",
|
||||
);
|
||||
if (failure) {
|
||||
await releaseTuiUpdateGate();
|
||||
return failure;
|
||||
}
|
||||
} else {
|
||||
@@ -316,6 +324,7 @@ export async function updateGitCheckout(params: {
|
||||
"checkout-failed",
|
||||
);
|
||||
if (failure) {
|
||||
await releaseTuiUpdateGate();
|
||||
return failure;
|
||||
}
|
||||
createdAtSelectedSha = !hasLocalMain;
|
||||
@@ -335,7 +344,9 @@ export async function updateGitCheckout(params: {
|
||||
"checkout-failed",
|
||||
);
|
||||
if (upstreamFailure) {
|
||||
return await rollbackError("checkout-failed");
|
||||
const rollbackFailure = await rollbackError("checkout-failed");
|
||||
await releaseTuiUpdateGate();
|
||||
return rollbackFailure;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -363,6 +374,7 @@ export async function updateGitCheckout(params: {
|
||||
totalSteps: 1,
|
||||
results: steps,
|
||||
});
|
||||
await releaseTuiUpdateGate();
|
||||
return buildError("rebase-failed");
|
||||
}
|
||||
}
|
||||
@@ -387,6 +399,7 @@ export async function updateGitCheckout(params: {
|
||||
"checkout-failed",
|
||||
);
|
||||
if (failure) {
|
||||
await releaseTuiUpdateGate();
|
||||
return failure;
|
||||
}
|
||||
}
|
||||
@@ -399,7 +412,9 @@ export async function updateGitCheckout(params: {
|
||||
"require-preferred",
|
||||
);
|
||||
if (manager.kind === "missing-required") {
|
||||
return await rollbackError(mapManagerResolutionFailure(manager.reason));
|
||||
const failure = await rollbackError(mapManagerResolutionFailure(manager.reason));
|
||||
await releaseTuiUpdateGate();
|
||||
return failure;
|
||||
}
|
||||
try {
|
||||
const installEnv = resolveInstallEnv(manager.manager, manager.env);
|
||||
@@ -574,5 +589,6 @@ export async function updateGitCheckout(params: {
|
||||
};
|
||||
} finally {
|
||||
await manager.cleanup?.();
|
||||
await releaseTuiUpdateGate();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -748,6 +748,8 @@ export async function withEmbeddedTuiStateLock<T>(
|
||||
}
|
||||
|
||||
export async function runTui(opts: RunTuiOptions): Promise<TuiResult> {
|
||||
const { waitForLocalTuiUpdate } = await import("../infra/local-tui-processes.js");
|
||||
await waitForLocalTuiUpdate();
|
||||
if (opts.local === true && opts.backend === undefined) {
|
||||
return await withEmbeddedTuiStateLock(async () => await runTuiUnlocked(opts));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user