mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
fix: /restart restarts the gateway over and over instead of once (#127179)
The /restart command is dispatched from a durable channel ingress row. The gateway begins its restart drain before that dispatch returns, so the row is still claimed when root-work admission closes. The drain sees GatewayDrainingError and releases the row without spending its retry budget (#125918), which is correct for an ordinary message and wrong for a command that caused the drain: the successor gateway claims the same row and runs /restart again, forever. Every boot is clean, so the gateway looks healthy throughout. Adopt the ingress claim in the restart command handler before scheduling the restart. The command is not idempotent, so losing the acknowledgement is better than an unbounded restart loop. The drain's deferral branch is unchanged. Co-authored-by: Marvinthebored <262704729+Marvinthebored@users.noreply.github.com>
This commit is contained in:
@@ -186,6 +186,79 @@ describe("handleRestartCommand", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("adopts the durable ingress claim before scheduling the restart", async () => {
|
||||
const order: string[] = [];
|
||||
mocks.scheduleGatewaySigusr1Restart.mockImplementationOnce((_opts) => {
|
||||
order.push("schedule");
|
||||
return { scheduled: true };
|
||||
});
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
await handleRestartCommand(
|
||||
restartCommandParams({
|
||||
opts: {
|
||||
turnAdoptionLifecycle: {
|
||||
onAdopted: () => {
|
||||
order.push("adopt");
|
||||
},
|
||||
},
|
||||
} as HandleCommandsParams["opts"],
|
||||
}),
|
||||
true,
|
||||
);
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
|
||||
// Unadopted at restart => drain releases with recordAttempt:false and the
|
||||
// successor replays /restart, restarting again forever.
|
||||
expect(order).toEqual(["adopt", "schedule"]);
|
||||
});
|
||||
|
||||
it("adopts the durable ingress claim before the fallback restart path", async () => {
|
||||
const order: string[] = [];
|
||||
mocks.triggerOpenClawRestart.mockImplementationOnce(() => {
|
||||
order.push("trigger");
|
||||
return { ok: true, method: "launchctl" };
|
||||
});
|
||||
|
||||
await handleRestartCommand(
|
||||
restartCommandParams({
|
||||
opts: {
|
||||
turnAdoptionLifecycle: {
|
||||
onAdopted: () => {
|
||||
order.push("adopt");
|
||||
},
|
||||
},
|
||||
} as HandleCommandsParams["opts"],
|
||||
}),
|
||||
true,
|
||||
);
|
||||
|
||||
expect(order).toEqual(["adopt", "trigger"]);
|
||||
});
|
||||
|
||||
it("does not restart when ingress adoption was lost to another owner", async () => {
|
||||
await expect(
|
||||
handleRestartCommand(
|
||||
restartCommandParams({
|
||||
opts: {
|
||||
turnAdoptionLifecycle: {
|
||||
onAdopted: () => {
|
||||
throw new Error("ingress adoption lost: guillotined");
|
||||
},
|
||||
},
|
||||
} as HandleCommandsParams["opts"],
|
||||
}),
|
||||
true,
|
||||
),
|
||||
).rejects.toThrow("ingress adoption lost");
|
||||
|
||||
expect(mocks.triggerOpenClawRestart).not.toHaveBeenCalled();
|
||||
expect(mocks.scheduleGatewaySigusr1Restart).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects authorized non-owner restart commands", async () => {
|
||||
const result = await handleRestartCommand(
|
||||
restartCommandParams({
|
||||
|
||||
@@ -611,6 +611,13 @@ export const handleRestartCommand: CommandHandler = async (params, allowTextComm
|
||||
if (!isRestartEnabled(params.cfg)) {
|
||||
return sessionCommandReply("⚠️ /restart is disabled (commands.restart=false).");
|
||||
}
|
||||
// Restart tears this process down before the dispatch that carries /restart can
|
||||
// return, so the durable ingress claim would still be held when admission closes.
|
||||
// The drain then releases it without spending retry budget and the successor
|
||||
// replays /restart forever. Adopt here: the command is not idempotent, so losing
|
||||
// the acknowledgement beats an unbounded restart loop. Adoption loss throws,
|
||||
// which correctly aborts the restart because another owner holds the event.
|
||||
await params.opts?.turnAdoptionLifecycle?.onAdopted();
|
||||
const hasSigusr1Listener = process.listenerCount("SIGUSR1") > 0;
|
||||
const sentinelPayload = buildRestartCommandSentinel(params);
|
||||
if (hasSigusr1Listener) {
|
||||
|
||||
Reference in New Issue
Block a user