From 1b998c0d84de7aeb46b1de550dcf682e84f4d53e Mon Sep 17 00:00:00 2001 From: Marvinthebored Date: Sat, 22 Aug 2026 03:12:58 +0800 Subject: [PATCH] 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> --- .../reply/commands-session-restart.test.ts | 73 +++++++++++++++++++ src/auto-reply/reply/commands-session.ts | 7 ++ 2 files changed, 80 insertions(+) diff --git a/src/auto-reply/reply/commands-session-restart.test.ts b/src/auto-reply/reply/commands-session-restart.test.ts index 4e72991f6d17..658e15a61c8c 100644 --- a/src/auto-reply/reply/commands-session-restart.test.ts +++ b/src/auto-reply/reply/commands-session-restart.test.ts @@ -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({ diff --git a/src/auto-reply/reply/commands-session.ts b/src/auto-reply/reply/commands-session.ts index d66f725a2aa3..4265edb7a24f 100644 --- a/src/auto-reply/reply/commands-session.ts +++ b/src/auto-reply/reply/commands-session.ts @@ -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) {