mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-10 00:03:54 +00:00
fix(gateway): commit runtime snapshot on noop config reloads (#100586)
Commit noop-only reload plans through the runtime secrets activator so getRuntimeConfig readers observe edited config without a Gateway restart, and make heartbeat wakes read the current runtime config while schedule recalculation stays owned by updateConfig/restart-heartbeat. Fixes stale messages.visibleReplies / messages.groupChat.visibleReplies and all sibling none-classified keys. Supersedes #100321; thanks @Sedrak-Hovhannisyan for the report. Co-Authored-By: Codex <noreply@openai.com>
This commit is contained in:
@@ -698,6 +698,9 @@ function createReloaderHarness(
|
||||
const onConfigApplied = vi.fn(
|
||||
async (_plan: GatewayReloadPlan, _nextConfig: OpenClawConfig) => {},
|
||||
);
|
||||
const onNoopConfigCommit = vi.fn(
|
||||
async (_plan: GatewayReloadPlan, _nextConfig: OpenClawConfig) => {},
|
||||
);
|
||||
const onHotReload = vi.fn(async (_plan: GatewayReloadPlan, _nextConfig: OpenClawConfig) => {});
|
||||
const onRestart = vi.fn((_plan: GatewayReloadPlan, _nextConfig: OpenClawConfig) => {});
|
||||
let writeListener: ((event: ConfigWriteNotification) => void) | null = null;
|
||||
@@ -726,6 +729,7 @@ function createReloaderHarness(
|
||||
subscribeToWrites,
|
||||
onConfigChange,
|
||||
onConfigApplied,
|
||||
onNoopConfigCommit,
|
||||
onHotReload,
|
||||
onRestart,
|
||||
log,
|
||||
@@ -735,6 +739,7 @@ function createReloaderHarness(
|
||||
watcher,
|
||||
onConfigChange,
|
||||
onConfigApplied,
|
||||
onNoopConfigCommit,
|
||||
onHotReload,
|
||||
onRestart,
|
||||
log,
|
||||
@@ -812,6 +817,39 @@ describe("startGatewayConfigReloader", () => {
|
||||
await harness.reloader.stop();
|
||||
});
|
||||
|
||||
it("commits runtime snapshot changes for no-op visible reply reloads", async () => {
|
||||
const initialConfig: OpenClawConfig = {
|
||||
gateway: { reload: { debounceMs: 0 } },
|
||||
messages: { visibleReplies: "automatic" },
|
||||
};
|
||||
const nextConfig: OpenClawConfig = {
|
||||
gateway: { reload: { debounceMs: 0 } },
|
||||
messages: { visibleReplies: "message_tool" },
|
||||
};
|
||||
const readSnapshot = vi.fn(async () =>
|
||||
makeSnapshot({ config: nextConfig, hash: "visible-replies" }),
|
||||
);
|
||||
const harness = createReloaderHarness(readSnapshot, { initialConfig });
|
||||
|
||||
harness.watcher.emit("change");
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(harness.onNoopConfigCommit).toHaveBeenCalledTimes(1);
|
||||
expect(harness.onNoopConfigCommit.mock.calls[0]?.[0].noopPaths).toContain(
|
||||
"messages.visibleReplies",
|
||||
);
|
||||
expect(harness.onNoopConfigCommit.mock.calls[0]?.[1]).toBe(nextConfig);
|
||||
expect(harness.onHotReload).not.toHaveBeenCalled();
|
||||
expect(harness.onRestart).not.toHaveBeenCalled();
|
||||
expect(harness.onConfigChange.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
harness.onNoopConfigCommit.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY,
|
||||
);
|
||||
expect(harness.onNoopConfigCommit.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
harness.onConfigApplied.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY,
|
||||
);
|
||||
await harness.reloader.stop();
|
||||
});
|
||||
|
||||
it("notifies lifecycle owners before hot reload and commits after success", async () => {
|
||||
const initialConfig: OpenClawConfig = {
|
||||
gateway: { reload: { debounceMs: 0 } },
|
||||
@@ -1774,6 +1812,7 @@ describe("startGatewayConfigReloader watcher error recovery", () => {
|
||||
readSnapshot: vi.fn(async () => makeSnapshot()),
|
||||
initialPluginInstallRecords: {},
|
||||
readPluginInstallRecords: async () => ({}),
|
||||
onNoopConfigCommit: vi.fn(async () => {}),
|
||||
onHotReload: vi.fn(async () => {}),
|
||||
onRestart: vi.fn(),
|
||||
log,
|
||||
|
||||
@@ -117,6 +117,7 @@ export function startGatewayConfigReloader(opts: {
|
||||
readSnapshot: () => Promise<ConfigFileSnapshot>;
|
||||
onConfigChange?: (plan: GatewayReloadPlan, nextConfig: OpenClawConfig) => void | Promise<void>;
|
||||
onConfigApplied?: (plan: GatewayReloadPlan, nextConfig: OpenClawConfig) => void | Promise<void>;
|
||||
onNoopConfigCommit: (plan: GatewayReloadPlan, nextConfig: OpenClawConfig) => Promise<void>;
|
||||
onHotReload: (plan: GatewayReloadPlan, nextConfig: OpenClawConfig) => Promise<void>;
|
||||
onRestart: (plan: GatewayReloadPlan, nextConfig: OpenClawConfig) => void | Promise<void>;
|
||||
promoteSnapshot?: (snapshot: ConfigFileSnapshot, reason: string) => Promise<boolean>;
|
||||
@@ -287,6 +288,9 @@ export function startGatewayConfigReloader(opts: {
|
||||
}
|
||||
if (isNoopReloadPlan(plan) && !followUp.requiresRestart) {
|
||||
await opts.onConfigChange?.(plan, nextConfig);
|
||||
// No-op plans still change the runtime config snapshot. Commit before
|
||||
// marking applied so getRuntimeConfig() readers do not stay stale until restart.
|
||||
await opts.onNoopConfigCommit(plan, nextConfig);
|
||||
await opts.onConfigApplied?.(plan, nextConfig);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1269,6 +1269,117 @@ describe("gateway Gmail hot reload handlers", () => {
|
||||
expect(clearGmailRestartAbortController).toHaveBeenCalledWith(abortController);
|
||||
});
|
||||
|
||||
it("commits runtime secrets for managed no-op config reloads", async () => {
|
||||
vi.useFakeTimers();
|
||||
const writeListenerRef: { current: ((event: ConfigWriteNotification) => void) | null } = {
|
||||
current: null,
|
||||
};
|
||||
const initialConfig: OpenClawConfig = {
|
||||
gateway: { reload: { debounceMs: 0 } },
|
||||
messages: { visibleReplies: "automatic" },
|
||||
};
|
||||
const nextConfig: OpenClawConfig = {
|
||||
gateway: { reload: { debounceMs: 0 } },
|
||||
messages: { visibleReplies: "message_tool" },
|
||||
};
|
||||
const activateRuntimeSecrets = vi.fn(async (config: OpenClawConfig) => ({
|
||||
sourceConfig: config,
|
||||
config,
|
||||
authStores: [],
|
||||
warnings: [],
|
||||
webTools: {},
|
||||
}));
|
||||
const heartbeatRunner = { stop: vi.fn(), updateConfig: vi.fn() };
|
||||
const reloader = startManagedGatewayConfigReloader({
|
||||
minimalTestGateway: false,
|
||||
initialConfig,
|
||||
initialCompareConfig: initialConfig,
|
||||
initialInternalWriteHash: null,
|
||||
watchPath: "/tmp/openclaw.json",
|
||||
readSnapshot: vi.fn(async () => ({
|
||||
path: "/tmp/openclaw.json",
|
||||
exists: true,
|
||||
raw: "{}",
|
||||
parsed: {},
|
||||
sourceConfig: nextConfig,
|
||||
resolved: nextConfig,
|
||||
valid: true,
|
||||
runtimeConfig: nextConfig,
|
||||
config: nextConfig,
|
||||
issues: [],
|
||||
warnings: [],
|
||||
legacyIssues: [],
|
||||
hash: "hash-next",
|
||||
})) as never,
|
||||
promoteSnapshot: vi.fn(async () => true) as never,
|
||||
subscribeToWrites: ((listener: (event: ConfigWriteNotification) => void) => {
|
||||
writeListenerRef.current = listener;
|
||||
return () => {
|
||||
if (writeListenerRef.current === listener) {
|
||||
writeListenerRef.current = null;
|
||||
}
|
||||
};
|
||||
}) as never,
|
||||
deps: {} as never,
|
||||
broadcast: vi.fn(),
|
||||
getState: () => ({
|
||||
hooksConfig: {} as never,
|
||||
hookClientIpConfig: {} as never,
|
||||
heartbeatRunner: heartbeatRunner as never,
|
||||
cronState: {
|
||||
cron: { start: vi.fn(async () => {}), stop: vi.fn() },
|
||||
storePath: "/tmp/cron.json",
|
||||
cronEnabled: false,
|
||||
} as never,
|
||||
channelHealthMonitor: null,
|
||||
}),
|
||||
setState: vi.fn(),
|
||||
startChannel: vi.fn(async () => {}),
|
||||
stopChannel: vi.fn(async () => {}),
|
||||
reloadPlugins: vi.fn(
|
||||
async (): Promise<GatewayPluginReloadResult> => ({
|
||||
restartChannels: new Set(),
|
||||
activeChannels: new Set(),
|
||||
}),
|
||||
),
|
||||
logHooks: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
logChannels: { info: vi.fn(), error: vi.fn() },
|
||||
logCron: { error: vi.fn() },
|
||||
logReload: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
channelManager: {} as never,
|
||||
activateRuntimeSecrets: activateRuntimeSecrets as never,
|
||||
resolveSharedGatewaySessionGenerationForConfig: () => undefined,
|
||||
sharedGatewaySessionGenerationState: { current: undefined, required: null },
|
||||
clients: [],
|
||||
reconcileTerminalSessions: vi.fn(),
|
||||
commitTerminalConfig: vi.fn(),
|
||||
});
|
||||
const registeredWriteListener = writeListenerRef.current;
|
||||
if (!registeredWriteListener) {
|
||||
throw new Error("Expected config write listener to be registered");
|
||||
}
|
||||
|
||||
registeredWriteListener({
|
||||
configPath: "/tmp/openclaw.json",
|
||||
sourceConfig: nextConfig,
|
||||
runtimeConfig: nextConfig,
|
||||
persistedHash: "hash-next",
|
||||
revision: 1,
|
||||
fingerprint: "runtime-hash-next",
|
||||
sourceFingerprint: "source-hash-next",
|
||||
writtenAtMs: Date.now(),
|
||||
});
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(activateRuntimeSecrets).toHaveBeenCalledTimes(1);
|
||||
expect(activateRuntimeSecrets).toHaveBeenCalledWith(nextConfig, {
|
||||
reason: "reload",
|
||||
activate: true,
|
||||
});
|
||||
expect(heartbeatRunner.updateConfig).not.toHaveBeenCalled();
|
||||
await reloader.stop();
|
||||
});
|
||||
|
||||
it("aborts an in-flight managed Gmail restart when the reloader stops", async () => {
|
||||
const writeListenerRef: { current: ((event: ConfigWriteNotification) => void) | null } = {
|
||||
current: null,
|
||||
|
||||
@@ -740,6 +740,12 @@ export function startManagedGatewayConfigReloader(params: ManagedGatewayConfigRe
|
||||
subscribeToWrites: params.subscribeToWrites,
|
||||
onConfigChange: (plan, nextConfig) => params.reconcileTerminalSessions(plan, nextConfig),
|
||||
onConfigApplied: () => params.commitTerminalConfig(),
|
||||
onNoopConfigCommit: async (_plan, nextConfig) => {
|
||||
await params.activateRuntimeSecrets(nextConfig, {
|
||||
reason: "reload",
|
||||
activate: true,
|
||||
});
|
||||
},
|
||||
onHotReload: async (plan, nextConfig) => {
|
||||
const previousSharedGatewaySessionGeneration =
|
||||
params.sharedGatewaySessionGenerationState.current;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
// Gateway post-ready runtime services.
|
||||
// Starts delayed maintenance, cron, heartbeat, recovery, and pricing refresh work.
|
||||
import { getRuntimeConfig } from "../config/config.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { isVitestRuntimeEnv } from "../infra/env.js";
|
||||
import { startHeartbeatRunner, type HeartbeatRunner } from "../infra/heartbeat-runner.js";
|
||||
@@ -228,7 +229,10 @@ export function activateGatewayScheduledServices(params: {
|
||||
// production starts without launching background loops.
|
||||
return { heartbeatRunner: createNoopHeartbeatRunner(), stopModelPricingRefresh: () => {} };
|
||||
}
|
||||
const heartbeatRunner = startHeartbeatRunner({ cfg: params.cfgAtStart });
|
||||
const heartbeatRunner = startHeartbeatRunner({
|
||||
cfg: params.cfgAtStart,
|
||||
readCurrentConfig: getRuntimeConfig,
|
||||
});
|
||||
if (params.startCron !== false) {
|
||||
startGatewayCronWithLogging({
|
||||
cron: params.cron,
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
// Tests heartbeat runner scheduling and timer cleanup.
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import {
|
||||
getRuntimeConfig,
|
||||
resetConfigRuntimeState,
|
||||
setRuntimeConfigSnapshot,
|
||||
type OpenClawConfig,
|
||||
} from "../config/config.js";
|
||||
import { startHeartbeatRunner } from "./heartbeat-runner.js";
|
||||
import { computeNextHeartbeatPhaseDueMs, resolveHeartbeatPhaseMs } from "./heartbeat-schedule.js";
|
||||
import {
|
||||
@@ -165,6 +170,7 @@ describe("startHeartbeatRunner", () => {
|
||||
|
||||
afterEach(() => {
|
||||
resetHeartbeatWakeStateForTests();
|
||||
resetConfigRuntimeState();
|
||||
vi.useRealTimers();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
@@ -218,6 +224,37 @@ describe("startHeartbeatRunner", () => {
|
||||
runner.stop();
|
||||
});
|
||||
|
||||
it("reads the latest runtime config for heartbeat wakes after no-op reload commits", async () => {
|
||||
useFakeHeartbeatTime();
|
||||
|
||||
const initialConfig: OpenClawConfig = {
|
||||
...heartbeatConfig(),
|
||||
messages: { visibleReplies: "automatic" },
|
||||
};
|
||||
const nextConfig: OpenClawConfig = {
|
||||
...heartbeatConfig(),
|
||||
messages: { visibleReplies: "message_tool" },
|
||||
};
|
||||
setRuntimeConfigSnapshot(initialConfig, initialConfig);
|
||||
const runSpy = vi.fn().mockResolvedValue({ status: "ran", durationMs: 1 });
|
||||
const runner = startHeartbeatRunner({
|
||||
cfg: initialConfig,
|
||||
readCurrentConfig: getRuntimeConfig,
|
||||
runOnce: runSpy,
|
||||
stableSchedulerSeed: TEST_SCHEDULER_SEED,
|
||||
});
|
||||
|
||||
setRuntimeConfigSnapshot(nextConfig, nextConfig);
|
||||
requestHeartbeat(wake("manual", { coalesceMs: 0 }));
|
||||
await vi.advanceTimersByTimeAsync(1);
|
||||
|
||||
expect(runSpy).toHaveBeenCalledTimes(1);
|
||||
const options = getRunCall(runSpy, 0);
|
||||
expect((options.cfg as OpenClawConfig).messages?.visibleReplies).toBe("message_tool");
|
||||
expect((options.heartbeat as { every?: string }).every).toBe("30m");
|
||||
runner.stop();
|
||||
});
|
||||
|
||||
it("schedules every configured agent when only global heartbeat defaults exist", async () => {
|
||||
useFakeHeartbeatTime();
|
||||
|
||||
|
||||
@@ -2316,6 +2316,7 @@ export async function runHeartbeatOnce(opts: {
|
||||
|
||||
export function startHeartbeatRunner(opts: {
|
||||
cfg?: OpenClawConfig;
|
||||
readCurrentConfig?: () => OpenClawConfig;
|
||||
runtime?: RuntimeEnv;
|
||||
abortSignal?: AbortSignal;
|
||||
runOnce?: typeof runHeartbeatOnce;
|
||||
@@ -2331,6 +2332,7 @@ export function startHeartbeatRunner(opts: {
|
||||
timer: null as NodeJS.Timeout | null,
|
||||
stopped: false,
|
||||
};
|
||||
const readCurrentConfig = opts.readCurrentConfig ?? (() => state.cfg);
|
||||
let initialized = false;
|
||||
let heartbeatTimeoutOverflowWarned = false;
|
||||
|
||||
@@ -2570,6 +2572,7 @@ export function startHeartbeatRunner(opts: {
|
||||
const isInterval = reason === "interval";
|
||||
const startedAt = Date.now();
|
||||
const now = startedAt;
|
||||
const wakeConfig = readCurrentConfig();
|
||||
let ran = false;
|
||||
// Track retryable busy skips so we can skip re-arm in finally — the wake
|
||||
// layer handles retry for this case (DEFAULT_RETRY_MS = 1 s).
|
||||
@@ -2589,10 +2592,10 @@ export function startHeartbeatRunner(opts: {
|
||||
}
|
||||
try {
|
||||
const res = await runOnce({
|
||||
cfg: state.cfg,
|
||||
cfg: wakeConfig,
|
||||
agentId: targetAgent.agentId,
|
||||
heartbeat: resolveHeartbeatForWake({
|
||||
cfg: state.cfg,
|
||||
cfg: wakeConfig,
|
||||
agentId: targetAgent.agentId,
|
||||
configuredHeartbeat: targetAgent.heartbeat,
|
||||
requestedHeartbeat,
|
||||
@@ -2654,7 +2657,7 @@ export function startHeartbeatRunner(opts: {
|
||||
let res: HeartbeatRunResult;
|
||||
try {
|
||||
res = await runOnce({
|
||||
cfg: state.cfg,
|
||||
cfg: wakeConfig,
|
||||
agentId: agent.agentId,
|
||||
heartbeat: agent.heartbeat,
|
||||
source: params.source,
|
||||
@@ -2687,13 +2690,13 @@ export function startHeartbeatRunner(opts: {
|
||||
let agentRan = res.status === "ran";
|
||||
|
||||
const defaultSessionKey = resolveHeartbeatSession(
|
||||
state.cfg,
|
||||
wakeConfig,
|
||||
agent.agentId,
|
||||
agent.heartbeat,
|
||||
).sessionKey;
|
||||
const dueSessionKeys = canHeartbeatDeliverCommitments(agent.heartbeat)
|
||||
? await listDueCommitmentSessionKeys({
|
||||
cfg: state.cfg,
|
||||
cfg: wakeConfig,
|
||||
agentId: agent.agentId,
|
||||
nowMs: now,
|
||||
limit: 10,
|
||||
@@ -2706,7 +2709,7 @@ export function startHeartbeatRunner(opts: {
|
||||
let commitmentRes: HeartbeatRunResult;
|
||||
try {
|
||||
commitmentRes = await runOnce({
|
||||
cfg: state.cfg,
|
||||
cfg: wakeConfig,
|
||||
agentId: agent.agentId,
|
||||
heartbeat: agent.heartbeat,
|
||||
runScope: "commitment-only",
|
||||
|
||||
Reference in New Issue
Block a user