mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 04:47:03 -06:00
fix: gateway boot recovers from a dead-owner migration lease and names its breaker escape hatch (#114718)
* fix(gateway): reclaim startup migration leases whose owner process is gone * fix(gateway): name a runnable command in crash-loop breaker suppression messages * fix(gateway): scope the breaker recovery hint to the suppressed account
This commit is contained in:
committed by
GitHub
parent
fa6a93ac97
commit
a1a507ddaf
@@ -186,7 +186,8 @@ restart handling continues.
|
||||
resumes on a later boot after the unclean-boot window drains. Gateway logs
|
||||
look like:
|
||||
`channel autostart suppressed by crash-loop breaker; refusing automatic
|
||||
start for <channel>… Use channels.start to override.`
|
||||
start for <channel>… Start a channel manually with: openclaw gateway call
|
||||
channels.start --params '{"channel":"<id>"}'`
|
||||
|
||||
Operator recovery SOP:
|
||||
|
||||
|
||||
@@ -96,6 +96,7 @@ const writeDiagnosticStabilityBundleForFailureSync = vi.fn((_reason: string, _er
|
||||
path: "/tmp/openclaw-stability.json",
|
||||
}));
|
||||
const bootLifecycle = vi.hoisted(() => ({
|
||||
manualChannelStartHint: `Start a channel manually with: openclaw gateway call channels.start --params '{"channel":"<id>"}'`,
|
||||
decisions: [] as Array<{
|
||||
tripped: boolean;
|
||||
uncleanBoots: number;
|
||||
@@ -297,6 +298,7 @@ vi.mock("../../logging/diagnostic-stability-bundle.js", () => ({
|
||||
|
||||
vi.mock("../../infra/gateway-boot-lifecycle.js", () => ({
|
||||
GATEWAY_CRASH_LOOP_BREAKER_REASON: "gateway.crash_loop_breaker",
|
||||
formatGatewayCrashLoopManualChannelStartHint: () => bootLifecycle.manualChannelStartHint,
|
||||
GATEWAY_CRASH_LOOP_RECOVERED_REASON: "gateway.crash_loop_recovered",
|
||||
inspectGatewayCrashLoopBreaker: (env?: NodeJS.ProcessEnv, nowMs?: number) =>
|
||||
bootLifecycle.inspect(env, nowMs),
|
||||
@@ -449,7 +451,7 @@ describe("gateway run option collisions", () => {
|
||||
return callArg(startGatewayServer, index, 1) as {
|
||||
auth?: { mode?: string; token?: string; password?: string };
|
||||
bind?: string;
|
||||
channelAutostartSuppression?: { reason?: string };
|
||||
channelAutostartSuppression?: { reason?: string; message?: string };
|
||||
ambientEnvTriggers?: "allow" | "suppress";
|
||||
startupConfigSnapshotRead?: { snapshot?: Record<string, unknown> };
|
||||
startupStartedAt?: number;
|
||||
@@ -1570,6 +1572,9 @@ describe("gateway run option collisions", () => {
|
||||
expect(gatewayStartOptions(0).channelAutostartSuppression).toMatchObject({
|
||||
reason: "crash-loop-breaker",
|
||||
});
|
||||
expect(gatewayStartOptions(0).channelAutostartSuppression?.message).toContain(
|
||||
bootLifecycle.manualChannelStartHint,
|
||||
);
|
||||
expect(gatewayStartOptions(1).channelAutostartSuppression).toBeUndefined();
|
||||
expect(gatewayLogMessages.some((message) => message.includes("breaker recovered"))).toBe(true);
|
||||
});
|
||||
|
||||
@@ -39,6 +39,7 @@ import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import {
|
||||
completeGatewayBootLifecycle,
|
||||
GATEWAY_CRASH_LOOP_BREAKER_REASON,
|
||||
formatGatewayCrashLoopManualChannelStartHint,
|
||||
GATEWAY_CRASH_LOOP_RECOVERED_REASON,
|
||||
inspectGatewayCrashLoopBreaker,
|
||||
recordGatewayBootStart,
|
||||
@@ -1136,7 +1137,7 @@ async function runGatewayCommandOnce(opts: GatewayRunOpts, hooks: GatewayRunRunt
|
||||
}
|
||||
const message =
|
||||
`gateway restart-loop breaker tripped: ${crashLoopDecision.uncleanBoots} unclean boot(s) within ${crashLoopDecision.windowMs}ms; ` +
|
||||
"suppressing channel/provider account auto-start. Inspect the stability bundle and fix the startup crash before restarting the service.";
|
||||
`suppressing channel/provider account auto-start. Inspect the stability bundle and fix the startup crash before restarting the service. ${formatGatewayCrashLoopManualChannelStartHint()}`;
|
||||
channelAutostartSuppression = { reason: "crash-loop-breaker", message };
|
||||
gatewayLog.error(message);
|
||||
if (crashLoopDecision.shouldWriteStabilityBundle) {
|
||||
|
||||
@@ -12,6 +12,7 @@ import { startChannelApprovalHandlerBootstrap } from "../infra/approval-handler-
|
||||
import { type BackoffPolicy, sleepWithAbort } from "../infra/backoff.js";
|
||||
import { createTaskScopedChannelRuntime } from "../infra/channel-runtime-context.js";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import { formatGatewayCrashLoopManualChannelStartHint } from "../infra/gateway-boot-lifecycle.js";
|
||||
import { resetDirectoryCache } from "../infra/outbound/target-resolver.js";
|
||||
import {
|
||||
createSubsystemLogger,
|
||||
@@ -481,7 +482,7 @@ export function createChannelManager(opts: ChannelManagerOptions): ChannelManage
|
||||
// config reloads can undo the crash-loop breaker while operators inspect.
|
||||
const suffix = accountId ? ` account ${accountId}` : "";
|
||||
ensureChannelLog(channelId).warn?.(
|
||||
`channel autostart suppressed by crash-loop breaker; refusing automatic start for ${channelId}${suffix}. Use channels.start to override.`,
|
||||
`channel autostart suppressed by crash-loop breaker; refusing automatic start for ${channelId}${suffix}. ${formatGatewayCrashLoopManualChannelStartHint({ channelId, ...(accountId ? { accountId } : {}) })}`,
|
||||
);
|
||||
for (const id of accountIds) {
|
||||
setStoppedRuntime(channelId, id, {
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
GATEWAY_CRASH_LOOP_BREAKER_REASON,
|
||||
GATEWAY_CRASH_LOOP_RECOVERED_REASON,
|
||||
completeGatewayBootLifecycle,
|
||||
formatGatewayCrashLoopManualChannelStartHint,
|
||||
inspectGatewayCrashLoopBreaker,
|
||||
recordGatewayBootStart,
|
||||
} from "./gateway-boot-lifecycle.js";
|
||||
@@ -220,3 +221,25 @@ describe("gateway crash-loop breaker", () => {
|
||||
expect(rows).not.toContain("old");
|
||||
});
|
||||
});
|
||||
|
||||
describe("formatGatewayCrashLoopManualChannelStartHint", () => {
|
||||
it("uses a placeholder when no channel is known", () => {
|
||||
expect(formatGatewayCrashLoopManualChannelStartHint()).toContain(
|
||||
`--params '{"channel":"<id>"}'`,
|
||||
);
|
||||
});
|
||||
|
||||
it("names the channel being suppressed", () => {
|
||||
expect(formatGatewayCrashLoopManualChannelStartHint({ channelId: "telegram" })).toContain(
|
||||
`--params '{"channel":"telegram"}'`,
|
||||
);
|
||||
});
|
||||
|
||||
// Suppression is reported per account; omitting accountId would tell operators to run a command
|
||||
// that starts the channel's default account instead of the one the warning named.
|
||||
it("carries the account when suppression is account-scoped", () => {
|
||||
expect(
|
||||
formatGatewayCrashLoopManualChannelStartHint({ channelId: "telegram", accountId: "work" }),
|
||||
).toContain(`--params '{"channel":"telegram","accountId":"work"}'`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -22,6 +22,24 @@ const GATEWAY_BOOT_LOOP_WINDOW_MS = 5 * 60_000;
|
||||
const GATEWAY_BOOT_LIFECYCLE_RETENTION_MS = 24 * 60 * 60_000;
|
||||
export const GATEWAY_CRASH_LOOP_BREAKER_REASON = "gateway.crash_loop_breaker";
|
||||
export const GATEWAY_CRASH_LOOP_RECOVERED_REASON = "gateway.crash_loop_recovered";
|
||||
/**
|
||||
* The breaker never self-clears within its window, so every operator-facing surface must name the
|
||||
* manual override command instead of the internal RPC name. Account-scoped suppression must carry
|
||||
* its accountId: `channels.start` resolves an omitted account to the channel default, so a hint
|
||||
* without it would start a different account than the one the message named.
|
||||
*/
|
||||
export function formatGatewayCrashLoopManualChannelStartHint(target?: {
|
||||
channelId: string;
|
||||
accountId?: string;
|
||||
}): string {
|
||||
const params = target
|
||||
? JSON.stringify({
|
||||
channel: target.channelId,
|
||||
...(target.accountId ? { accountId: target.accountId } : {}),
|
||||
})
|
||||
: `{"channel":"<id>"}`;
|
||||
return `Start a channel manually with: openclaw gateway call channels.start --params '${params}'`;
|
||||
}
|
||||
|
||||
const gatewayLifecycleLog = createSubsystemLogger("gateway/lifecycle");
|
||||
|
||||
|
||||
@@ -3,11 +3,18 @@ import { existsSync, mkdirSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
|
||||
import type { DB as OpenClawStateKyselyDatabase } from "../state/openclaw-state-db.generated.js";
|
||||
import {
|
||||
closeOpenClawStateDatabaseForTest,
|
||||
OPENCLAW_STATE_SCHEMA_VERSION,
|
||||
withOpenClawStateStartupMigrationCheckpointDatabase,
|
||||
} from "../state/openclaw-state-db.js";
|
||||
import { resolveOpenClawStateSqlitePath } from "../state/openclaw-state-db.paths.js";
|
||||
import {
|
||||
executeSqliteQuerySync,
|
||||
executeSqliteQueryTakeFirstSync,
|
||||
getNodeSqliteKysely,
|
||||
} from "./kysely-sync.js";
|
||||
import { requireNodeSqlite } from "./node-sqlite.js";
|
||||
import {
|
||||
acquireStartupMigrationLease,
|
||||
@@ -23,6 +30,32 @@ afterEach(() => {
|
||||
|
||||
const startupMigrationTempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
|
||||
type StartupMigrationLeaseTestDatabase = Pick<OpenClawStateKyselyDatabase, "state_leases">;
|
||||
|
||||
/** Rewrites only the recorded owner start time so the live owner PID looks recycled. */
|
||||
function overwriteStartupMigrationLeaseOwnerStartedAt(
|
||||
env: NodeJS.ProcessEnv,
|
||||
startedAt: number,
|
||||
): void {
|
||||
withOpenClawStateStartupMigrationCheckpointDatabase(
|
||||
(db) => {
|
||||
const kysely = getNodeSqliteKysely<StartupMigrationLeaseTestDatabase>(db);
|
||||
const row = executeSqliteQueryTakeFirstSync(
|
||||
db,
|
||||
kysely.selectFrom("state_leases").select("payload_json as payloadJson"),
|
||||
);
|
||||
const payload = JSON.parse(row?.payloadJson ?? "{}") as { owner?: { startedAt?: number } };
|
||||
executeSqliteQuerySync(
|
||||
db,
|
||||
kysely.updateTable("state_leases").set({
|
||||
payload_json: JSON.stringify({ ...payload, owner: { ...payload.owner, startedAt } }),
|
||||
}),
|
||||
);
|
||||
},
|
||||
{ env },
|
||||
);
|
||||
}
|
||||
|
||||
describe("startup migration checkpoint", () => {
|
||||
it("checks migration activity without creating shared state", () => {
|
||||
const env = {
|
||||
@@ -112,7 +145,7 @@ describe("startup migration checkpoint", () => {
|
||||
expect(hasActiveStartupMigrationLease({ env, nowMs: 1001 })).toBe(true);
|
||||
|
||||
expect(() => acquireStartupMigrationLease({ env, nowMs: 1001, owner: "second" })).toThrow(
|
||||
"OpenClaw startup migrations are already running",
|
||||
`OpenClaw startup migrations are already running for this state directory; retry after the other gateway finishes or after 1970-01-01T00:05:01.000Z. (held by pid ${process.pid})`,
|
||||
);
|
||||
|
||||
lease.release();
|
||||
@@ -123,6 +156,48 @@ describe("startup migration checkpoint", () => {
|
||||
next.release();
|
||||
});
|
||||
|
||||
it("reclaims an active startup migration lease whose owner process is gone", () => {
|
||||
const env = {
|
||||
OPENCLAW_STATE_DIR: startupMigrationTempDirs.make("openclaw-startup-migration-"),
|
||||
};
|
||||
const deadPid = 2_147_483_647;
|
||||
const stale = acquireStartupMigrationLease({
|
||||
env,
|
||||
nowMs: 1000,
|
||||
owner: "stale",
|
||||
ownerPid: deadPid,
|
||||
});
|
||||
|
||||
expect(hasActiveStartupMigrationLease({ env, nowMs: 1001 })).toBe(false);
|
||||
|
||||
const replacement = acquireStartupMigrationLease({ env, nowMs: 1001, owner: "replacement" });
|
||||
stale.release();
|
||||
expect(hasActiveStartupMigrationLease({ env, nowMs: 1002 })).toBe(true);
|
||||
replacement.release();
|
||||
});
|
||||
|
||||
// PID numbers are recycled by the OS. Without the start-time guard a stale lease whose PID was
|
||||
// reassigned to an unrelated live process would block startup for the full TTL.
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"reclaims a startup migration lease whose owner PID was recycled",
|
||||
() => {
|
||||
const env = {
|
||||
OPENCLAW_STATE_DIR: startupMigrationTempDirs.make("openclaw-startup-migration-"),
|
||||
};
|
||||
const stale = acquireStartupMigrationLease({ env, nowMs: 1000, owner: "stale" });
|
||||
|
||||
// The owner PID is this live test process; only the recorded start identity is stale.
|
||||
overwriteStartupMigrationLeaseOwnerStartedAt(env, 1);
|
||||
|
||||
expect(hasActiveStartupMigrationLease({ env, nowMs: 1001 })).toBe(false);
|
||||
|
||||
const replacement = acquireStartupMigrationLease({ env, nowMs: 1001, owner: "replacement" });
|
||||
stale.release();
|
||||
expect(hasActiveStartupMigrationLease({ env, nowMs: 1002 })).toBe(true);
|
||||
replacement.release();
|
||||
},
|
||||
);
|
||||
|
||||
it("does not report an expired startup migration lease as active", () => {
|
||||
const env = {
|
||||
OPENCLAW_STATE_DIR: startupMigrationTempDirs.make("openclaw-startup-migration-"),
|
||||
|
||||
@@ -2,7 +2,10 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { existsSync } from "node:fs";
|
||||
import { createRequire } from "node:module";
|
||||
import { hostname } from "node:os";
|
||||
import type { DatabaseSync } from "node:sqlite";
|
||||
import { isRecord } from "@openclaw/normalization-core/record-coerce";
|
||||
import { getFileLockProcessStartTime, isPidDefinitelyDead } from "../shared/pid-alive.js";
|
||||
import { withOpenClawStateDatabaseReadOnly } from "../state/openclaw-state-db-readonly.js";
|
||||
import type { DB as OpenClawStateKyselyDatabase } from "../state/openclaw-state-db.generated.js";
|
||||
import { withOpenClawStateStartupMigrationCheckpointDatabase } from "../state/openclaw-state-db.js";
|
||||
@@ -32,6 +35,60 @@ export type StartupMigrationLease = {
|
||||
readonly owner: string;
|
||||
};
|
||||
|
||||
type StartupMigrationLeaseOwner = {
|
||||
pid: number;
|
||||
host: string;
|
||||
startedAt: number | null;
|
||||
};
|
||||
|
||||
function parseStartupMigrationLeaseOwner(
|
||||
payloadJson: string | null,
|
||||
): StartupMigrationLeaseOwner | null {
|
||||
if (!payloadJson) {
|
||||
return null;
|
||||
}
|
||||
let owner: unknown;
|
||||
try {
|
||||
const parsed: unknown = JSON.parse(payloadJson);
|
||||
owner = isRecord(parsed) ? parsed.owner : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
if (!isRecord(owner)) {
|
||||
return null;
|
||||
}
|
||||
const { pid, host, startedAt } = owner;
|
||||
if (
|
||||
typeof pid !== "number" ||
|
||||
!Number.isSafeInteger(pid) ||
|
||||
pid <= 0 ||
|
||||
typeof host !== "string" ||
|
||||
!host ||
|
||||
(startedAt !== null &&
|
||||
(typeof startedAt !== "number" || !Number.isSafeInteger(startedAt) || startedAt < 0))
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
return { pid, host, startedAt };
|
||||
}
|
||||
|
||||
function isStartupMigrationLeaseOwnerDefinitelyGone(
|
||||
owner: StartupMigrationLeaseOwner | null,
|
||||
): boolean {
|
||||
// Reclaim only same-host owners whose PID identity is provably gone.
|
||||
// The recorded start time prevents PID reuse from making a stale lease look live.
|
||||
if (!owner || owner.host !== hostname()) {
|
||||
return false;
|
||||
}
|
||||
if (isPidDefinitelyDead(owner.pid)) {
|
||||
return true;
|
||||
}
|
||||
const currentStartedAt = getFileLockProcessStartTime(owner.pid);
|
||||
return (
|
||||
owner.startedAt !== null && currentStartedAt !== null && currentStartedAt !== owner.startedAt
|
||||
);
|
||||
}
|
||||
|
||||
function formatStartupMigrationCheckpoint(version: string, buildIdentity: string): string {
|
||||
return `${version}${STARTUP_MIGRATION_BUILD_SEPARATOR}${buildIdentity}`;
|
||||
}
|
||||
@@ -111,15 +168,19 @@ export function hasActiveStartupMigrationLease(
|
||||
return withOpenClawStateDatabaseReadOnly(
|
||||
({ db }) => {
|
||||
const stateDb = getNodeSqliteKysely<StartupMigrationCheckpointDatabase>(db);
|
||||
const lease = executeSqliteQueryTakeFirstSync(
|
||||
db,
|
||||
stateDb
|
||||
.selectFrom("state_leases")
|
||||
.select("payload_json as payloadJson")
|
||||
.where("scope", "=", STARTUP_MIGRATION_LEASE_SCOPE)
|
||||
.where("lease_key", "=", STARTUP_MIGRATION_LEASE_KEY)
|
||||
.where("expires_at", ">", nowMs),
|
||||
);
|
||||
return Boolean(
|
||||
executeSqliteQueryTakeFirstSync(
|
||||
db,
|
||||
stateDb
|
||||
.selectFrom("state_leases")
|
||||
.select("owner")
|
||||
.where("scope", "=", STARTUP_MIGRATION_LEASE_SCOPE)
|
||||
.where("lease_key", "=", STARTUP_MIGRATION_LEASE_KEY)
|
||||
.where("expires_at", ">", nowMs),
|
||||
lease &&
|
||||
!isStartupMigrationLeaseOwnerDefinitelyGone(
|
||||
parseStartupMigrationLeaseOwner(lease.payloadJson),
|
||||
),
|
||||
);
|
||||
},
|
||||
@@ -153,11 +214,19 @@ export function acquireStartupMigrationLease(
|
||||
env?: NodeJS.ProcessEnv;
|
||||
nowMs?: number;
|
||||
owner?: string;
|
||||
/** Process id that owns the startup migration work. */
|
||||
ownerPid?: number;
|
||||
} = {},
|
||||
): StartupMigrationLease {
|
||||
const env = params.env ?? process.env;
|
||||
const nowMs = params.nowMs ?? Date.now();
|
||||
const owner = params.owner ?? randomUUID();
|
||||
const ownerPid = params.ownerPid ?? process.pid;
|
||||
const leaseOwner: StartupMigrationLeaseOwner = {
|
||||
pid: ownerPid,
|
||||
host: hostname(),
|
||||
startedAt: getFileLockProcessStartTime(ownerPid),
|
||||
};
|
||||
const expiresAt = nowMs + STARTUP_MIGRATION_LEASE_TTL_MS;
|
||||
|
||||
writeStartupMigrationCheckpointDatabase(env, (db) => {
|
||||
@@ -174,13 +243,24 @@ export function acquireStartupMigrationLease(
|
||||
db,
|
||||
stateDb
|
||||
.selectFrom("state_leases")
|
||||
.select(["owner", "expires_at as expiresAt"])
|
||||
.select(["owner", "expires_at as expiresAt", "payload_json as payloadJson"])
|
||||
.where("scope", "=", STARTUP_MIGRATION_LEASE_SCOPE)
|
||||
.where("lease_key", "=", STARTUP_MIGRATION_LEASE_KEY),
|
||||
);
|
||||
if (existing) {
|
||||
const existingOwner = parseStartupMigrationLeaseOwner(existing?.payloadJson ?? null);
|
||||
if (existing && isStartupMigrationLeaseOwnerDefinitelyGone(existingOwner)) {
|
||||
executeSqliteQuerySync(
|
||||
db,
|
||||
stateDb
|
||||
.deleteFrom("state_leases")
|
||||
.where("scope", "=", STARTUP_MIGRATION_LEASE_SCOPE)
|
||||
.where("lease_key", "=", STARTUP_MIGRATION_LEASE_KEY)
|
||||
.where("owner", "=", existing.owner),
|
||||
);
|
||||
} else if (existing) {
|
||||
const ownerHint = existingOwner ? ` (held by pid ${existingOwner.pid})` : "";
|
||||
throw new Error(
|
||||
`OpenClaw startup migrations are already running for this state directory; retry after the other gateway finishes or after ${new Date(existing.expiresAt ?? expiresAt).toISOString()}.`,
|
||||
`OpenClaw startup migrations are already running for this state directory; retry after the other gateway finishes or after ${new Date(existing.expiresAt ?? expiresAt).toISOString()}.${ownerHint}`,
|
||||
);
|
||||
}
|
||||
executeSqliteQuerySync(
|
||||
@@ -191,7 +271,7 @@ export function acquireStartupMigrationLease(
|
||||
owner,
|
||||
expires_at: expiresAt,
|
||||
heartbeat_at: nowMs,
|
||||
payload_json: JSON.stringify({ version: VERSION }),
|
||||
payload_json: JSON.stringify({ version: VERSION, owner: leaseOwner }),
|
||||
created_at: nowMs,
|
||||
updated_at: nowMs,
|
||||
}),
|
||||
|
||||
Reference in New Issue
Block a user