mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 05:16:23 -06:00
fix(infra/agents): session-routing guard for coalesced gateway restart continuations (#86742) (#87323)
* fix(infra/agents): session-routing guard for coalesced gateway restart continuations (#86742) When two sessions issue gateway.restart with continuationMessage close together, the scheduler Path B updatePendingRestartEmitHooks unconditionally overwrote the existing pending hooks, silently dropping the first sessions continuation and potentially routing the second sessions continuation back to the first session (CWE-200 finding flagged by aisle-research-bot on prior attempt #74443). Add a session-routing guard: scheduleGatewaySigusr1Restart now accepts an optional sessionKey and tracks the pending restarts owning session. Coalesced callers from a different session are rejected at the hook- update step and the new ScheduledRestart.emitHooksQueued: false field surfaces the drop to the caller. The gateway tool propagates this as continuationQueued: false in the tool response, matching #83370 narrow report-only surface. Same-session debounce/replace and legacy hookless callers behave the same as before. Refs #86742 * fix(infra): preserve queued restart continuation on forced bypass * fix(infra): make forced restart hook preservation explicit * fix(infra): guard restart continuation ownership before reschedule * fix(infra): report hookless coalesced restarts accurately * fix(infra): trust runtime session for restart sentinel routing * fix(infra): preserve earlier restart reschedule semantics * fix(agents): trust runtime session for update continuations * fix(infra): preserve hookless forced restart continuations --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -616,9 +616,9 @@ public struct SendParams: Codable, Sendable {
|
||||
message: String?,
|
||||
mediaurl: String?,
|
||||
mediaurls: [String]?,
|
||||
buffer: String?,
|
||||
filename: String?,
|
||||
contenttype: String?,
|
||||
buffer: String? = nil,
|
||||
filename: String? = nil,
|
||||
contenttype: String? = nil,
|
||||
asvoice: Bool?,
|
||||
gifplayback: Bool?,
|
||||
channel: String?,
|
||||
|
||||
@@ -42,6 +42,7 @@ const STRICT_LITERAL_STRUCTS = new Set([
|
||||
]);
|
||||
|
||||
const DEFAULTED_OPTIONAL_INIT_PARAM_ENTRIES: readonly [string, readonly string[]][] = [
|
||||
["SendParams", ["buffer", "filename", "contentType"]],
|
||||
["SessionOperationEvent", ["agentId"]],
|
||||
["SessionsCompactionListParams", ["agentId"]],
|
||||
["SessionsCompactionGetParams", ["agentId"]],
|
||||
|
||||
@@ -11,11 +11,13 @@ const {
|
||||
extractDeliveryInfoMock,
|
||||
formatDoctorNonInteractiveHintMock,
|
||||
isRestartEnabledMock,
|
||||
callGatewayToolMock,
|
||||
removeRestartSentinelFileMock,
|
||||
scheduleGatewaySigusr1RestartMock,
|
||||
writeRestartSentinelMock,
|
||||
} = vi.hoisted(() => ({
|
||||
isRestartEnabledMock: vi.fn(() => true),
|
||||
callGatewayToolMock: vi.fn(async () => ({ ok: true })),
|
||||
extractDeliveryInfoMock: vi.fn(() => ({
|
||||
deliveryContext: {
|
||||
channel: "slack",
|
||||
@@ -31,8 +33,14 @@ const {
|
||||
writeRestartSentinelMock: vi.fn(async (_payload: RestartSentinelPayload) => "/tmp/restart"),
|
||||
removeRestartSentinelFileMock: vi.fn(async (_path: string | null | undefined) => undefined),
|
||||
scheduleGatewaySigusr1RestartMock: vi.fn((_opts?: ScheduleGatewayRestartArgs) => ({
|
||||
scheduled: true,
|
||||
ok: true,
|
||||
pid: 123,
|
||||
signal: "SIGUSR1" as const,
|
||||
delayMs: 250,
|
||||
mode: "emit" as const,
|
||||
coalesced: false,
|
||||
cooldownMsApplied: 0,
|
||||
emitHooksQueued: true,
|
||||
})),
|
||||
}));
|
||||
|
||||
@@ -67,7 +75,7 @@ vi.mock("../../logging/subsystem.js", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("./gateway.js", () => ({
|
||||
callGatewayTool: vi.fn(),
|
||||
callGatewayTool: callGatewayToolMock,
|
||||
readGatewayCallOptions: vi.fn(() => ({})),
|
||||
}));
|
||||
|
||||
@@ -110,7 +118,18 @@ describe("gateway tool restart continuation", () => {
|
||||
writeRestartSentinelMock.mockResolvedValue("/tmp/restart");
|
||||
removeRestartSentinelFileMock.mockClear();
|
||||
scheduleGatewaySigusr1RestartMock.mockReset();
|
||||
scheduleGatewaySigusr1RestartMock.mockReturnValue({ scheduled: true, delayMs: 250 });
|
||||
scheduleGatewaySigusr1RestartMock.mockReturnValue({
|
||||
ok: true,
|
||||
pid: 123,
|
||||
signal: "SIGUSR1",
|
||||
delayMs: 250,
|
||||
mode: "emit",
|
||||
coalesced: false,
|
||||
cooldownMsApplied: 0,
|
||||
emitHooksQueued: true,
|
||||
});
|
||||
callGatewayToolMock.mockReset();
|
||||
callGatewayToolMock.mockResolvedValue({ ok: true });
|
||||
});
|
||||
|
||||
it("does not expose system-event continuations to the agent tool", async () => {
|
||||
@@ -187,9 +206,62 @@ describe("gateway tool restart continuation", () => {
|
||||
const restartArgs = requireScheduledRestartArgs();
|
||||
expect(restartArgs.delayMs).toBe(250);
|
||||
expect(restartArgs.reason).toBe("continue after reboot");
|
||||
expect(restartArgs.sessionKey).toBe("agent:main:main");
|
||||
expect(typeof restartArgs.emitHooks?.beforeEmit).toBe("function");
|
||||
expect(typeof restartArgs.emitHooks?.afterEmitRejected).toBe("function");
|
||||
expect(result?.details).toEqual({ scheduled: true, delayMs: 250 });
|
||||
expect(result?.details).toMatchObject({
|
||||
ok: true,
|
||||
delayMs: 250,
|
||||
coalesced: false,
|
||||
emitHooksQueued: true,
|
||||
continuationQueued: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("uses the runtime session, not model-supplied params, for scheduler ownership and sentinel routing (#86742)", async () => {
|
||||
const tool = createGatewayTool({
|
||||
agentSessionKey: "agent:main:session-A",
|
||||
config: {},
|
||||
});
|
||||
|
||||
await tool.execute?.("tool-call-1", {
|
||||
action: "restart",
|
||||
sessionKey: "agent:main:session-B",
|
||||
continuationMessage: "Reply after restart",
|
||||
});
|
||||
|
||||
expect(requireScheduledRestartArgs().sessionKey).toBe("agent:main:session-A");
|
||||
await requireScheduledRestartArgs().emitHooks?.beforeEmit?.();
|
||||
expect(requireRestartSentinelPayload().sessionKey).toBe("agent:main:session-A");
|
||||
});
|
||||
|
||||
it("reports continuationQueued=false when a coalesced restart belongs to another session (#86742)", async () => {
|
||||
scheduleGatewaySigusr1RestartMock.mockReturnValue({
|
||||
ok: true,
|
||||
pid: 123,
|
||||
signal: "SIGUSR1",
|
||||
delayMs: 0,
|
||||
mode: "emit",
|
||||
coalesced: true,
|
||||
cooldownMsApplied: 0,
|
||||
emitHooksQueued: false,
|
||||
});
|
||||
const tool = createGatewayTool({
|
||||
agentSessionKey: "agent:main:main",
|
||||
config: {},
|
||||
});
|
||||
|
||||
const result = await tool.execute?.("tool-call-1", {
|
||||
action: "restart",
|
||||
continuationMessage: "Reply after restart",
|
||||
});
|
||||
|
||||
expect(writeRestartSentinelMock).not.toHaveBeenCalled();
|
||||
expect(result?.details).toMatchObject({
|
||||
coalesced: true,
|
||||
emitHooksQueued: false,
|
||||
continuationQueued: false,
|
||||
});
|
||||
});
|
||||
|
||||
it.each([-1, 1.5, "soon"])("rejects invalid restart delayMs value %s", async (delayMs) => {
|
||||
@@ -278,4 +350,30 @@ describe("gateway tool restart continuation", () => {
|
||||
|
||||
expect(removeRestartSentinelFileMock).toHaveBeenCalledWith("/tmp/restart");
|
||||
});
|
||||
|
||||
it("uses the runtime session for update.run continuation routing (#86742)", async () => {
|
||||
const tool = createGatewayTool({
|
||||
agentSessionKey: "agent:main:session-A",
|
||||
config: {},
|
||||
});
|
||||
|
||||
await tool.execute?.("tool-call-update", {
|
||||
action: "update.run",
|
||||
sessionKey: "agent:main:session-B",
|
||||
continuationMessage: "Reply after update restart",
|
||||
note: "Updating now",
|
||||
restartDelayMs: 0,
|
||||
});
|
||||
|
||||
expect(callGatewayToolMock).toHaveBeenCalledWith(
|
||||
"update.run",
|
||||
expect.objectContaining({ timeoutMs: expect.any(Number) }),
|
||||
expect.objectContaining({
|
||||
sessionKey: "agent:main:session-A",
|
||||
continuationMessage: "Reply after update restart",
|
||||
note: "Updating now",
|
||||
restartDelayMs: 0,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -395,8 +395,8 @@ export function createGatewayTool(opts?: {
|
||||
throw new Error("Gateway restart is disabled (commands.restart=false).");
|
||||
}
|
||||
const sessionKey =
|
||||
normalizeOptionalString(params.sessionKey) ??
|
||||
normalizeOptionalString(opts?.agentSessionKey);
|
||||
normalizeOptionalString(opts?.agentSessionKey) ??
|
||||
normalizeOptionalString(params.sessionKey);
|
||||
const delayMs = readNonNegativeIntegerParam(params, "delayMs");
|
||||
const reason = normalizeOptionalString(params.reason)?.slice(0, 200);
|
||||
const note = normalizeOptionalString(params.note);
|
||||
@@ -429,6 +429,9 @@ export function createGatewayTool(opts?: {
|
||||
const scheduled = scheduleGatewaySigusr1Restart({
|
||||
delayMs,
|
||||
reason,
|
||||
// Ownership and sentinel routing use the same trusted session identity,
|
||||
// so model-supplied params cannot queue work into another session.
|
||||
sessionKey,
|
||||
emitHooks: {
|
||||
beforeEmit: async () => {
|
||||
sentinelPath = await writeRestartSentinel(payload);
|
||||
@@ -438,7 +441,10 @@ export function createGatewayTool(opts?: {
|
||||
},
|
||||
},
|
||||
});
|
||||
return jsonResult(scheduled);
|
||||
return jsonResult({
|
||||
...scheduled,
|
||||
...(payload.continuation ? { continuationQueued: scheduled.emitHooksQueued } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
const gatewayOpts = readGatewayCallOptions(params);
|
||||
@@ -449,8 +455,8 @@ export function createGatewayTool(opts?: {
|
||||
restartDelayMs: number | undefined;
|
||||
} => {
|
||||
const sessionKey =
|
||||
normalizeOptionalString(params.sessionKey) ??
|
||||
normalizeOptionalString(opts?.agentSessionKey);
|
||||
normalizeOptionalString(opts?.agentSessionKey) ??
|
||||
normalizeOptionalString(params.sessionKey);
|
||||
const note = normalizeOptionalString(params.note);
|
||||
const restartDelayMs = readNonNegativeIntegerParam(params, "restartDelayMs");
|
||||
return { sessionKey, note, restartDelayMs };
|
||||
|
||||
@@ -180,6 +180,18 @@ describe("handleRestartCommand", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("threads sessionKey into scheduleGatewaySigusr1Restart so cross-session coalescing is rejected (#86742)", async () => {
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
await handleRestartCommand(restartCommandParams(), true);
|
||||
const scheduledArgs = mocks.scheduleGatewaySigusr1Restart.mock.calls.at(-1)?.[0];
|
||||
expect(scheduledArgs?.sessionKey).toBe("agent:main:telegram:direct:123:thread:thread-1");
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects authorized non-owner restart commands", async () => {
|
||||
const result = await handleRestartCommand(
|
||||
restartCommandParams({
|
||||
|
||||
@@ -706,6 +706,10 @@ export const handleRestartCommand: CommandHandler = async (params, allowTextComm
|
||||
let sentinelPath: string | null = null;
|
||||
scheduleGatewaySigusr1Restart({
|
||||
reason: "/restart",
|
||||
// Sibling session-routing guard: /restart writes a session-scoped sentinel
|
||||
// with continuation, so the scheduler must own the pending slot under the
|
||||
// same key to avoid cross-session continuation overwrite (#86742).
|
||||
sessionKey: sentinelPayload?.sessionKey,
|
||||
emitHooks: sentinelPayload
|
||||
? {
|
||||
beforeEmit: async () => {
|
||||
|
||||
@@ -324,6 +324,179 @@ describe("infra runtime", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("reports emitHooksQueued=false for hookless coalesced restart requests", () => {
|
||||
const first = scheduleGatewaySigusr1Restart({ delayMs: 1_000, reason: "first" });
|
||||
const second = scheduleGatewaySigusr1Restart({ delayMs: 1_000, reason: "second" });
|
||||
|
||||
expect(first.coalesced).toBe(false);
|
||||
expect(first.emitHooksQueued).toBe(false);
|
||||
expect(second.coalesced).toBe(true);
|
||||
expect(second.emitHooksQueued).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects coalesced emit hooks from a different session and reports emitHooksQueued=false (#86742)", async () => {
|
||||
const sessionAHooks = vi.fn(async () => {});
|
||||
const sessionBHooks = vi.fn(async () => {});
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
const first = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
reason: "session-A",
|
||||
sessionKey: "agent:main:session-A",
|
||||
emitHooks: { beforeEmit: sessionAHooks },
|
||||
});
|
||||
const second = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
reason: "session-B",
|
||||
sessionKey: "agent:main:session-B",
|
||||
emitHooks: { beforeEmit: sessionBHooks },
|
||||
});
|
||||
|
||||
expect(first.coalesced).toBe(false);
|
||||
expect(first.emitHooksQueued).toBe(true);
|
||||
expect(second.coalesced).toBe(true);
|
||||
expect(second.emitHooksQueued).toBe(false);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
|
||||
// Session A's hook ran (it owns the pending slot); session B's hook is dropped,
|
||||
// which the caller already observed via emitHooksQueued=false.
|
||||
expect(sessionAHooks).toHaveBeenCalledTimes(1);
|
||||
expect(sessionBHooks).not.toHaveBeenCalled();
|
||||
expect(emitSpy).toHaveBeenCalledWith("SIGUSR1");
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
});
|
||||
|
||||
it("allows same-session coalesced restart to replace its own preparation hook (#86742)", async () => {
|
||||
const firstHooks = vi.fn(async () => {});
|
||||
const latestHooks = vi.fn(async () => {});
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
const first = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
reason: "first",
|
||||
sessionKey: "agent:main:session-A",
|
||||
emitHooks: { beforeEmit: firstHooks },
|
||||
});
|
||||
const second = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
reason: "second",
|
||||
sessionKey: "agent:main:session-A",
|
||||
emitHooks: { beforeEmit: latestHooks },
|
||||
});
|
||||
|
||||
expect(first.emitHooksQueued).toBe(true);
|
||||
expect(second.coalesced).toBe(true);
|
||||
expect(second.emitHooksQueued).toBe(true);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
|
||||
// Same session: latest hooks take ownership (existing debounce semantics).
|
||||
expect(firstHooks).not.toHaveBeenCalled();
|
||||
expect(latestHooks).toHaveBeenCalledTimes(1);
|
||||
expect(emitSpy).toHaveBeenCalledWith("SIGUSR1");
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects earlier reschedule hooks from a different session (#86742)", async () => {
|
||||
const sessionAHooks = vi.fn(async () => {});
|
||||
const sessionBHooks = vi.fn(async () => {});
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
const first = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
reason: "session-A",
|
||||
sessionKey: "agent:main:session-A",
|
||||
emitHooks: { beforeEmit: sessionAHooks },
|
||||
});
|
||||
const second = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 0,
|
||||
reason: "session-B",
|
||||
sessionKey: "agent:main:session-B",
|
||||
emitHooks: { beforeEmit: sessionBHooks },
|
||||
});
|
||||
|
||||
expect(first.coalesced).toBe(false);
|
||||
expect(first.emitHooksQueued).toBe(true);
|
||||
expect(second.coalesced).toBe(true);
|
||||
expect(second.emitHooksQueued).toBe(false);
|
||||
expect(second.delayMs).toBe(0);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
expect(sessionAHooks).toHaveBeenCalledTimes(1);
|
||||
expect(sessionBHooks).not.toHaveBeenCalled();
|
||||
expect(emitSpy).toHaveBeenCalledWith("SIGUSR1");
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects coalesced emit hooks from a different session while preparation is in flight (#86742)", async () => {
|
||||
// Pins the CWE-200 in-flight preparation race: pendingRestartSessionKey
|
||||
// must stay alive through await beforeEmit(), otherwise a coalesced
|
||||
// different-session caller slips past updatePendingRestartEmitHooks
|
||||
// and chains its own hooks while preparation runs.
|
||||
let releaseSessionAPrep: () => void = () => {};
|
||||
const sessionAPrepBlocked = new Promise<void>((resolve) => {
|
||||
releaseSessionAPrep = resolve;
|
||||
});
|
||||
const sessionAHooks = vi.fn(async () => {
|
||||
await sessionAPrepBlocked;
|
||||
});
|
||||
const sessionBHooks = vi.fn(async () => {});
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
const first = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
reason: "session-A",
|
||||
sessionKey: "agent:main:session-A",
|
||||
emitHooks: { beforeEmit: sessionAHooks },
|
||||
});
|
||||
// Advance the scheduled timer so session A's beforeEmit starts and
|
||||
// pendingRestartPreparing becomes true; the hook awaits forever until
|
||||
// we resolve sessionAPrepBlocked below.
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
await Promise.resolve();
|
||||
expect(sessionAHooks).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Session B coalesces *during* session A's beforeEmit await window.
|
||||
const second = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
reason: "session-B",
|
||||
sessionKey: "agent:main:session-B",
|
||||
emitHooks: { beforeEmit: sessionBHooks },
|
||||
});
|
||||
|
||||
expect(first.emitHooksQueued).toBe(true);
|
||||
expect(second.coalesced).toBe(true);
|
||||
expect(second.emitHooksQueued).toBe(false);
|
||||
|
||||
releaseSessionAPrep();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
|
||||
// Session B's hook must NOT run — the guard kept session A as owner
|
||||
// through the in-flight preparation window.
|
||||
expect(sessionBHooks).not.toHaveBeenCalled();
|
||||
expect(emitSpy).toHaveBeenCalledWith("SIGUSR1");
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps existing preparation hook when a hookless restart coalesces", async () => {
|
||||
const beforeEmit = vi.fn(async () => {});
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
@@ -595,6 +768,39 @@ describe("infra runtime", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves a session-owned preparation hook when a hookless forced restart pulls a pending timer earlier (#86742)", async () => {
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const beforeEmit = vi.fn(async () => {});
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
const pending = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
reason: "session-A",
|
||||
sessionKey: "agent:main:session-A",
|
||||
emitHooks: { beforeEmit },
|
||||
});
|
||||
const forced = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 1_000,
|
||||
preservePendingEmitHooksOnDeferralBypass: true,
|
||||
reason: "gateway.restart.safe",
|
||||
skipDeferral: true,
|
||||
});
|
||||
|
||||
expect(pending.emitHooksQueued).toBe(true);
|
||||
expect(forced.coalesced).toBe(false);
|
||||
expect(forced.emitHooksQueued).toBe(false);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
|
||||
expect(beforeEmit).toHaveBeenCalledTimes(1);
|
||||
expect(emitSpy).toHaveBeenCalledWith("SIGUSR1");
|
||||
expect(peekGatewaySigusr1RestartReason()).toBe("gateway.restart.safe");
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
});
|
||||
|
||||
it("bypasses an active restart deferral when a forced restart arrives", async () => {
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const staleBeforeEmit = vi.fn(async () => {});
|
||||
@@ -625,6 +831,76 @@ describe("infra runtime", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("clears a session-owned preparation hook when a forced update owns the sentinel (#86742)", async () => {
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const beforeEmit = vi.fn(async () => {});
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
setPreRestartDeferralCheck(() => 5);
|
||||
scheduleGatewaySigusr1Restart({
|
||||
delayMs: 0,
|
||||
reason: "session-A",
|
||||
sessionKey: "agent:main:session-A",
|
||||
emitHooks: { beforeEmit },
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
expect(emitSpy).not.toHaveBeenCalledWith("SIGUSR1");
|
||||
|
||||
const forced = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 0,
|
||||
reason: "update.run",
|
||||
skipDeferral: true,
|
||||
});
|
||||
|
||||
expect(forced.coalesced).toBe(false);
|
||||
expect(forced.emitHooksQueued).toBe(false);
|
||||
expect(beforeEmit).not.toHaveBeenCalled();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(emitSpy).toHaveBeenCalledWith("SIGUSR1");
|
||||
expect(peekGatewaySigusr1RestartReason()).toBe("update.run");
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves a session-owned preparation hook when a hookless forced restart bypasses active deferral (#86742)", async () => {
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const beforeEmit = vi.fn(async () => {});
|
||||
const handler = () => {};
|
||||
process.on("SIGUSR1", handler);
|
||||
try {
|
||||
setPreRestartDeferralCheck(() => 5);
|
||||
const pending = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 0,
|
||||
reason: "session-A",
|
||||
sessionKey: "agent:main:session-A",
|
||||
emitHooks: { beforeEmit },
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
expect(emitSpy).not.toHaveBeenCalledWith("SIGUSR1");
|
||||
|
||||
const forced = scheduleGatewaySigusr1Restart({
|
||||
delayMs: 0,
|
||||
preservePendingEmitHooksOnDeferralBypass: true,
|
||||
reason: "gateway.restart.safe",
|
||||
skipDeferral: true,
|
||||
});
|
||||
|
||||
expect(pending.emitHooksQueued).toBe(true);
|
||||
expect(forced.coalesced).toBe(false);
|
||||
expect(forced.emitHooksQueued).toBe(false);
|
||||
expect(beforeEmit).toHaveBeenCalledTimes(1);
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(emitSpy).toHaveBeenCalledWith("SIGUSR1");
|
||||
expect(peekGatewaySigusr1RestartReason()).toBe("gateway.restart.safe");
|
||||
} finally {
|
||||
process.removeListener("SIGUSR1", handler);
|
||||
}
|
||||
});
|
||||
|
||||
it("emits SIGUSR1 after the default deferral timeout while work is still pending", async () => {
|
||||
const emitSpy = vi.spyOn(process, "emit");
|
||||
const handler = () => {};
|
||||
|
||||
@@ -145,6 +145,7 @@ describe("safe gateway restart coordinator", () => {
|
||||
expect(result.preflight.safe).toBe(false);
|
||||
expect(scheduleGatewaySigusr1Restart).toHaveBeenCalledWith({
|
||||
delayMs: 0,
|
||||
preservePendingEmitHooksOnDeferralBypass: true,
|
||||
reason: "test.skip-deferral",
|
||||
skipDeferral: true,
|
||||
});
|
||||
|
||||
@@ -164,6 +164,7 @@ export function requestSafeGatewayRestart(
|
||||
const restart = scheduleGatewaySigusr1Restart({
|
||||
delayMs: opts.delayMs ?? 0,
|
||||
reason: opts.reason ?? "gateway.restart.safe",
|
||||
...(skipDeferral ? { preservePendingEmitHooksOnDeferralBypass: true } : {}),
|
||||
...(skipDeferral ? { skipDeferral: true } : {}),
|
||||
});
|
||||
const status = restart.coalesced
|
||||
|
||||
+135
-33
@@ -47,6 +47,7 @@ let pendingRestartTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
let pendingRestartDueAt = 0;
|
||||
let pendingRestartReason: string | undefined;
|
||||
let pendingRestartEmitHooks: RestartEmitHooks | undefined;
|
||||
let pendingRestartSessionKey: string | undefined;
|
||||
let pendingRestartSkipDeferral = false;
|
||||
let pendingRestartPreparing = false;
|
||||
const activeDeferralPolls = new Set<ReturnType<typeof setInterval>>();
|
||||
@@ -67,10 +68,41 @@ function clearPendingScheduledRestart(): void {
|
||||
pendingRestartDueAt = 0;
|
||||
pendingRestartReason = undefined;
|
||||
pendingRestartEmitHooks = undefined;
|
||||
pendingRestartSessionKey = undefined;
|
||||
pendingRestartSkipDeferral = false;
|
||||
pendingRestartPreparing = false;
|
||||
}
|
||||
|
||||
function armPendingRestartTimer(requestedDueAt: number, nowMs: number): void {
|
||||
pendingRestartTimer = setTimeout(
|
||||
() => {
|
||||
const scheduledReason = pendingRestartReason;
|
||||
const scheduledSkipDeferral = pendingRestartSkipDeferral;
|
||||
pendingRestartTimer = null;
|
||||
pendingRestartDueAt = 0;
|
||||
pendingRestartReason = undefined;
|
||||
pendingRestartSkipDeferral = false;
|
||||
pendingRestartPreparing = true;
|
||||
const pendingCheck = preRestartCheck;
|
||||
if (scheduledSkipDeferral || !pendingCheck) {
|
||||
void emitPreparedGatewayRestart(undefined, scheduledReason);
|
||||
return;
|
||||
}
|
||||
const cfg = getRuntimeConfig();
|
||||
const deferralTimeoutMs = resolveGatewayRestartDeferralTimeoutMs(
|
||||
cfg.gateway?.reload?.deferralTimeoutMs,
|
||||
);
|
||||
deferGatewayRestartUntilIdle({
|
||||
getPendingCount: pendingCheck,
|
||||
maxWaitMs: deferralTimeoutMs,
|
||||
reason: scheduledReason,
|
||||
timeoutIntent: { force: true, ...(scheduledReason ? { reason: scheduledReason } : {}) },
|
||||
});
|
||||
},
|
||||
Math.max(0, requestedDueAt - nowMs),
|
||||
);
|
||||
}
|
||||
|
||||
function clearActiveDeferralPolls(): void {
|
||||
for (const poll of activeDeferralPolls) {
|
||||
clearInterval(poll);
|
||||
@@ -440,10 +472,34 @@ export function resolveGatewayRestartDeferralTimeoutMs(timeoutMs: unknown): numb
|
||||
return Math.floor(timeoutMs);
|
||||
}
|
||||
|
||||
function updatePendingRestartEmitHooks(hooks?: RestartEmitHooks): void {
|
||||
if (hooks) {
|
||||
pendingRestartEmitHooks = hooks;
|
||||
function canReplacePendingRestartEmitHooks(
|
||||
hooks: RestartEmitHooks | undefined,
|
||||
sessionKey: string | undefined,
|
||||
): boolean {
|
||||
if (!hooks) {
|
||||
return true;
|
||||
}
|
||||
return pendingRestartSessionKey === undefined || pendingRestartSessionKey === sessionKey;
|
||||
}
|
||||
|
||||
// Returns true when the new hooks took ownership of the pending restart slot.
|
||||
// Coalesced callers from a different sessionKey are rejected to prevent the
|
||||
// cross-session continuation overwrite documented in #86742 (CWE-200).
|
||||
function updatePendingRestartEmitHooks(
|
||||
hooks: RestartEmitHooks | undefined,
|
||||
sessionKey: string | undefined,
|
||||
): boolean {
|
||||
if (!canReplacePendingRestartEmitHooks(hooks, sessionKey)) {
|
||||
return false;
|
||||
}
|
||||
if (!hooks) {
|
||||
return false;
|
||||
}
|
||||
pendingRestartEmitHooks = hooks;
|
||||
if (sessionKey !== undefined) {
|
||||
pendingRestartSessionKey = sessionKey;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
async function emitPreparedGatewayRestart(
|
||||
@@ -452,6 +508,10 @@ async function emitPreparedGatewayRestart(
|
||||
intent?: GatewayRestartIntent,
|
||||
): Promise<void> {
|
||||
let nextHooks = hooks ?? pendingRestartEmitHooks;
|
||||
// Keep pendingRestartSessionKey alive across the await beforeEmit() window:
|
||||
// a different-session caller that coalesces while preparation runs would
|
||||
// otherwise slip past the updatePendingRestartEmitHooks session guard and
|
||||
// chain its own hooks (#86742, the CWE-200 in-flight preparation race).
|
||||
if (!hooks) {
|
||||
pendingRestartEmitHooks = undefined;
|
||||
}
|
||||
@@ -475,6 +535,9 @@ async function emitPreparedGatewayRestart(
|
||||
nextHooks = pendingRestartEmitHooks;
|
||||
pendingRestartEmitHooks = undefined;
|
||||
}
|
||||
if (!hooks) {
|
||||
pendingRestartSessionKey = undefined;
|
||||
}
|
||||
|
||||
const emitted = emitGatewayRestart(reasonOverride, intent);
|
||||
if (!emitted) {
|
||||
@@ -716,6 +779,10 @@ export type ScheduledRestart = {
|
||||
mode: "emit" | "signal" | "supervisor";
|
||||
coalesced: boolean;
|
||||
cooldownMsApplied: number;
|
||||
// True iff the caller's emitHooks own the pending restart slot. Coalesced
|
||||
// requests from a different sessionKey are rejected to protect the existing
|
||||
// session's continuation (#86742).
|
||||
emitHooksQueued: boolean;
|
||||
};
|
||||
|
||||
export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
@@ -723,6 +790,8 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
reason?: string;
|
||||
audit?: RestartAuditInfo;
|
||||
emitHooks?: RestartEmitHooks;
|
||||
preservePendingEmitHooksOnDeferralBypass?: boolean;
|
||||
sessionKey?: string;
|
||||
skipDeferral?: boolean;
|
||||
skipCooldown?: boolean;
|
||||
}): ScheduledRestart {
|
||||
@@ -744,6 +813,8 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
: Math.max(0, lastRestartEmittedAt + RESTART_COOLDOWN_MS - nowMs);
|
||||
const requestedDueAt = nowMs + delayMs + cooldownMsApplied;
|
||||
const skipDeferral = opts?.skipDeferral === true;
|
||||
let nextPendingEmitHooks = opts?.emitHooks;
|
||||
let nextPendingSessionKey = opts?.sessionKey;
|
||||
|
||||
if (hasUnconsumedRestartSignal()) {
|
||||
if (shouldPreferRestartReason(reason, emittedRestartReason)) {
|
||||
@@ -765,6 +836,8 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
mode,
|
||||
coalesced: true,
|
||||
cooldownMsApplied,
|
||||
// SIGUSR1 already emitted; the new caller's hooks cannot run for this cycle.
|
||||
emitHooksQueued: false,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -776,7 +849,16 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
);
|
||||
clearActiveDeferralPolls();
|
||||
pendingRestartReason = reason;
|
||||
pendingRestartEmitHooks = opts?.emitHooks;
|
||||
// Hookless forced restarts that own no sentinel may preserve an accepted
|
||||
// pending hook; update/handoff callers rely on the default clear path.
|
||||
const preservePendingHooks =
|
||||
opts?.preservePendingEmitHooksOnDeferralBypass === true &&
|
||||
opts?.emitHooks === undefined &&
|
||||
pendingRestartSessionKey !== undefined;
|
||||
if (!preservePendingHooks) {
|
||||
pendingRestartEmitHooks = opts?.emitHooks;
|
||||
pendingRestartSessionKey = opts?.sessionKey;
|
||||
}
|
||||
void emitPreparedGatewayRestart(undefined, reason);
|
||||
return {
|
||||
ok: true,
|
||||
@@ -787,6 +869,7 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
mode,
|
||||
coalesced: false,
|
||||
cooldownMsApplied,
|
||||
emitHooksQueued: opts?.emitHooks !== undefined,
|
||||
};
|
||||
}
|
||||
const shouldUpgradeToSkipDeferral = skipDeferral && !pendingRestartSkipDeferral;
|
||||
@@ -794,10 +877,47 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
!pendingRestartPreparing &&
|
||||
(requestedDueAt < pendingRestartDueAt || shouldUpgradeToSkipDeferral);
|
||||
if (shouldPullEarlier) {
|
||||
const preservePendingHooks =
|
||||
opts?.preservePendingEmitHooksOnDeferralBypass === true &&
|
||||
opts?.emitHooks === undefined &&
|
||||
pendingRestartSessionKey !== undefined;
|
||||
if (
|
||||
!preservePendingHooks &&
|
||||
!canReplacePendingRestartEmitHooks(opts?.emitHooks, opts?.sessionKey)
|
||||
) {
|
||||
restartLog.warn(
|
||||
`restart continuation dropped: another session owns the pending restart (callerSessionKey=${opts?.sessionKey ?? "unspecified"} pendingSessionKey=${pendingRestartSessionKey ?? "unspecified"})`,
|
||||
);
|
||||
if (pendingRestartTimer) {
|
||||
clearTimeout(pendingRestartTimer);
|
||||
}
|
||||
pendingRestartTimer = null;
|
||||
pendingRestartDueAt = requestedDueAt;
|
||||
pendingRestartReason = reason;
|
||||
pendingRestartSkipDeferral = pendingRestartSkipDeferral || skipDeferral;
|
||||
armPendingRestartTimer(requestedDueAt, nowMs);
|
||||
return {
|
||||
ok: true,
|
||||
pid: process.pid,
|
||||
signal: "SIGUSR1",
|
||||
delayMs: Math.max(0, requestedDueAt - nowMs),
|
||||
reason,
|
||||
mode,
|
||||
coalesced: true,
|
||||
cooldownMsApplied,
|
||||
emitHooksQueued: false,
|
||||
};
|
||||
}
|
||||
const preservedEmitHooks = preservePendingHooks ? pendingRestartEmitHooks : undefined;
|
||||
const preservedSessionKey = preservePendingHooks ? pendingRestartSessionKey : undefined;
|
||||
restartLog.warn(
|
||||
`restart request rescheduled earlier reason=${reason ?? "unspecified"} pendingReason=${pendingRestartReason ?? "unspecified"} oldDelayMs=${remainingMs} newDelayMs=${Math.max(0, requestedDueAt - nowMs)} ${formatRestartAudit(opts?.audit)}`,
|
||||
);
|
||||
clearPendingScheduledRestart();
|
||||
if (preservePendingHooks) {
|
||||
nextPendingEmitHooks = preservedEmitHooks;
|
||||
nextPendingSessionKey = preservedSessionKey;
|
||||
}
|
||||
} else {
|
||||
if (shouldPreferRestartReason(reason, pendingRestartReason)) {
|
||||
pendingRestartReason = reason;
|
||||
@@ -806,7 +926,12 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
restartLog.warn(
|
||||
`restart request coalesced (already scheduled) reason=${reason ?? "unspecified"} pendingReason=${pendingRestartReason ?? "unspecified"} delayMs=${remainingMs} ${formatRestartAudit(opts?.audit)}`,
|
||||
);
|
||||
updatePendingRestartEmitHooks(opts?.emitHooks);
|
||||
const emitHooksQueued = updatePendingRestartEmitHooks(opts?.emitHooks, opts?.sessionKey);
|
||||
if (opts?.emitHooks && !emitHooksQueued) {
|
||||
restartLog.warn(
|
||||
`restart continuation dropped: another session owns the pending restart (callerSessionKey=${opts.sessionKey ?? "unspecified"} pendingSessionKey=${pendingRestartSessionKey ?? "unspecified"})`,
|
||||
);
|
||||
}
|
||||
return {
|
||||
ok: true,
|
||||
pid: process.pid,
|
||||
@@ -816,41 +941,17 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
mode,
|
||||
coalesced: true,
|
||||
cooldownMsApplied,
|
||||
emitHooksQueued,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
pendingRestartDueAt = requestedDueAt;
|
||||
pendingRestartReason = reason;
|
||||
pendingRestartEmitHooks = opts?.emitHooks;
|
||||
pendingRestartEmitHooks = nextPendingEmitHooks;
|
||||
pendingRestartSessionKey = nextPendingSessionKey;
|
||||
pendingRestartSkipDeferral = skipDeferral;
|
||||
pendingRestartTimer = setTimeout(
|
||||
() => {
|
||||
const scheduledReason = pendingRestartReason;
|
||||
const scheduledSkipDeferral = pendingRestartSkipDeferral;
|
||||
pendingRestartTimer = null;
|
||||
pendingRestartDueAt = 0;
|
||||
pendingRestartReason = undefined;
|
||||
pendingRestartSkipDeferral = false;
|
||||
pendingRestartPreparing = true;
|
||||
const pendingCheck = preRestartCheck;
|
||||
if (scheduledSkipDeferral || !pendingCheck) {
|
||||
void emitPreparedGatewayRestart(undefined, scheduledReason);
|
||||
return;
|
||||
}
|
||||
const cfg = getRuntimeConfig();
|
||||
const deferralTimeoutMs = resolveGatewayRestartDeferralTimeoutMs(
|
||||
cfg.gateway?.reload?.deferralTimeoutMs,
|
||||
);
|
||||
deferGatewayRestartUntilIdle({
|
||||
getPendingCount: pendingCheck,
|
||||
maxWaitMs: deferralTimeoutMs,
|
||||
reason: scheduledReason,
|
||||
timeoutIntent: { force: true, ...(scheduledReason ? { reason: scheduledReason } : {}) },
|
||||
});
|
||||
},
|
||||
Math.max(0, requestedDueAt - nowMs),
|
||||
);
|
||||
armPendingRestartTimer(requestedDueAt, nowMs);
|
||||
return {
|
||||
ok: true,
|
||||
pid: process.pid,
|
||||
@@ -860,6 +961,7 @@ export function scheduleGatewaySigusr1Restart(opts?: {
|
||||
mode,
|
||||
coalesced: false,
|
||||
cooldownMsApplied,
|
||||
emitHooksQueued: opts?.emitHooks !== undefined,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user