fix(gateway): retain Bonjour cleanup on startup failure (#127062)

This commit is contained in:
Peter Steinberger
2026-08-20 23:39:12 -07:00
committed by GitHub
parent 750f2f3762
commit 72c8bf9946
5 changed files with 77 additions and 11 deletions
+2 -4
View File
@@ -186,6 +186,7 @@ export async function startGatewayCoreRuntime(input: {
log,
logDiscovery,
nodeRegistry,
swapBonjourStop: kernel.swapBonjourStop,
pluginRegistry: pluginRuntime.registry,
broadcast,
nodeSendToAllSubscribed,
@@ -226,10 +227,7 @@ export async function startGatewayCoreRuntime(input: {
const discoveryResident = residentRegistry.register({
name: "bonjour-discovery",
start: startEarlyRuntime,
stop: async () => {
const earlyRuntime = await startEarlyRuntime();
await earlyRuntime.bonjourStop?.();
},
stop: async () => await kernel.swapBonjourStop(null)?.(),
});
const taskAndSkillsResident = residentRegistry.register({
name: "task-and-skills-runtime",
+18 -1
View File
@@ -20,7 +20,7 @@ import { createSyntheticPluginRuntimeClient } from "./server-plugin-runtime-clie
describe("createGatewayKernel", () => {
it("reports startup and readiness as draining during a direct close", async () => {
const port = await getFreePort();
const port = 19_789;
const state = await createOpenClawTestState({
label: "gateway-kernel-direct-close-readiness",
layout: "home",
@@ -56,6 +56,20 @@ describe("createGatewayKernel", () => {
expect(getStartup()).toMatchObject({ ok: true, status: "started" });
expect(getReadiness()).toMatchObject({ ready: true, failing: [] });
const discoveryResident = kernel.residentRegistry
.list()
.find((resident) => resident.name === "bonjour-discovery");
if (!discoveryResident) {
throw new Error("Expected the Gateway discovery resident");
}
const residentFirstStop = vi.fn(async () => {});
kernel.kernel.swapBonjourStop(residentFirstStop);
await discoveryResident.stop();
expect(residentFirstStop).toHaveBeenCalledOnce();
expect(kernel.runtimeState.bonjourStop).toBeNull();
const closeFirstStop = vi.fn(async () => {});
kernel.kernel.swapBonjourStop(closeFirstStop);
const configReloaderStop = createDeferred();
vi.spyOn(kernel.runtimeState.configReloader, "stop").mockReturnValue(
configReloaderStop.promise,
@@ -66,6 +80,9 @@ describe("createGatewayKernel", () => {
expect(getReadiness()).toMatchObject({ ready: false, failing: ["gateway-draining"] });
configReloaderStop.resolve();
await closing;
await discoveryResident.stop();
expect(closeFirstStop).toHaveBeenCalledOnce();
expect(kernel.runtimeState.bonjourStop).toBeNull();
} finally {
try {
await kernel?.closeOnStartupFailure();
+1 -3
View File
@@ -250,11 +250,9 @@ export async function prepareGatewayLifecycle(params: {
runtimeState.gatewayMethods.splice(0, runtimeState.gatewayMethods.length, ...methods);
},
setEarlyRuntimeHandles: (handles: {
bonjourStop: typeof runtimeState.bonjourStop;
getActiveTaskCount: () => number;
skillsChangeUnsub: typeof runtimeState.skillsChangeUnsub;
}) => {
runtimeState.bonjourStop = handles.bonjourStop;
activeTaskCount.get = handles.getActiveTaskCount;
runtimeState.skillsChangeUnsub = handles.skillsChangeUnsub;
},
@@ -515,7 +513,7 @@ export async function prepareGatewayLifecycle(params: {
const transport = transportBridge.current();
await transport?.portalService.closeAll();
await shutdownRuntime.createGatewayCloseHandler({
bonjourStop: runtimeState.bonjourStop,
bonjourStop: kernel.swapBonjourStop(null),
tailscaleCleanup: runtimeState.tailscaleCleanup,
clearSecretsRuntimeSnapshot: clearSecretsRuntimeSnapshotState,
channelIds,
+50
View File
@@ -2,6 +2,7 @@
* Early gateway startup helper tests.
*/
import { beforeEach, describe, expect, it, vi } from "vitest";
import { runGatewayShutdownSteps } from "./server-shutdown.js";
import { createGatewayMaintenanceStateForTest } from "./test-helpers.maintenance-state.js";
type StartGatewayDiscovery = typeof import("./server-discovery-runtime.js").startGatewayDiscovery;
@@ -82,6 +83,7 @@ function earlyRuntimeInput(
log,
logDiscovery: log,
nodeRegistry: {} as never,
swapBonjourStop: () => null,
...maintenanceState,
skillsRefreshDelayMs: 30_000,
getSkillsRefreshTimer: () => null,
@@ -150,6 +152,48 @@ describe("startGatewayEarlyRuntime", () => {
expect(mocks.closeSkillsWatchers).toHaveBeenCalledTimes(1);
});
it.each([false, true])(
"stops acquired discovery exactly once after later startup failure (cleanup rejects: %s)",
async (cleanupRejects) => {
const startupError = new Error("remote skills registry failed");
const cleanupError = new Error("discovery cleanup failed");
const stopDiscovery = vi.fn(async () => {
if (cleanupRejects) {
throw cleanupError;
}
});
const owner: { current: (() => Promise<void>) | null } = { current: null };
const swapBonjourStop = (next: typeof owner.current) => {
const previous = owner.current;
owner.current = next;
return previous;
};
mocks.startGatewayDiscovery.mockResolvedValueOnce({ bonjourStop: stopDiscovery });
mocks.setSkillsRemoteRegistry.mockImplementationOnce(() => {
throw startupError;
});
const onCleanupError = vi.fn();
const startup = startGatewayEarlyRuntime(
earlyRuntimeInput({ minimalTestGateway: false, swapBonjourStop }),
).catch(async (error: unknown) => {
await runGatewayShutdownSteps({
steps: [
{ name: "discovery resident", run: async () => await swapBonjourStop(null)?.() },
{ name: "gateway close", run: async () => await swapBonjourStop(null)?.() },
],
onError: onCleanupError,
});
throw error;
});
await expect(startup).rejects.toBe(startupError);
expect(stopDiscovery).toHaveBeenCalledOnce();
expect(owner.current).toBeNull();
expect(onCleanupError).toHaveBeenCalledTimes(cleanupRejects ? 1 : 0);
},
);
it("broadcasts remote-node skill invalidations to operator clients", async () => {
const broadcast = vi.fn();
@@ -212,6 +256,9 @@ describe("startGatewayEarlyRuntime", () => {
});
it("fails before discovery and task maintenance when task state cannot restore", async () => {
const stopDiscovery = vi.fn(async () => {});
const swapBonjourStop = vi.fn(() => null);
mocks.startGatewayDiscovery.mockResolvedValue({ bonjourStop: stopDiscovery });
mocks.ensureTaskRuntimeStateReady.mockImplementationOnce(() => {
throw new Error("task-flow registry restore failed");
});
@@ -220,11 +267,14 @@ describe("startGatewayEarlyRuntime", () => {
startGatewayEarlyRuntime(
earlyRuntimeInput({
minimalTestGateway: false,
swapBonjourStop,
}),
),
).rejects.toThrow("task-flow registry restore failed");
expect(mocks.startGatewayDiscovery).not.toHaveBeenCalled();
expect(swapBonjourStop).not.toHaveBeenCalled();
expect(stopDiscovery).not.toHaveBeenCalled();
expect(mocks.configureTaskRegistryMaintenance).not.toHaveBeenCalled();
expect(mocks.startTaskRegistryMaintenance).not.toHaveBeenCalled();
});
+6 -3
View File
@@ -71,6 +71,7 @@ export async function startGatewayEarlyRuntime(params: {
warn: (msg: string) => void;
};
nodeRegistry: Parameters<typeof import("../skills/runtime/remote.js").setSkillsRemoteRegistry>[0];
swapBonjourStop: (next: (() => Promise<void>) | null) => (() => Promise<void>) | null;
pluginRegistry?: PluginRegistry;
broadcast: GatewayMaintenanceParams["broadcast"];
nodeSendToAllSubscribed: Parameters<StartGatewayMaintenanceTimers>[0]["nodeSendToAllSubscribed"];
@@ -102,8 +103,11 @@ export async function startGatewayEarlyRuntime(params: {
ensureTaskRuntimeStateReady();
});
}
const bonjourStop = await measureStartup(params.startupTrace, "runtime.early.discovery", () =>
startGatewayPluginDiscovery(params),
// Startup failure can occur immediately after discovery; publish its owner first.
params.swapBonjourStop(
await measureStartup(params.startupTrace, "runtime.early.discovery", () =>
startGatewayPluginDiscovery(params),
),
);
let getActiveTaskCount = () => 0;
@@ -205,7 +209,6 @@ export async function startGatewayEarlyRuntime(params: {
};
return {
bonjourStop,
getActiveTaskCount,
skillsChangeUnsub,
startMaintenance,