diff --git a/src/config/io.logging.test.ts b/src/config/io.logging.test.ts new file mode 100644 index 00000000000..0de0a477ec1 --- /dev/null +++ b/src/config/io.logging.test.ts @@ -0,0 +1,105 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createConfigIO } from "./io.js"; + +async function withTempConfig( + run: (params: { + home: string; + configPath: string; + logger: { warn: ReturnType; error: ReturnType }; + loadConfig: () => Record; + }) => Promise, +): Promise { + const home = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-log-")); + const configDir = path.join(home, ".openclaw"); + const configPath = path.join(configDir, "openclaw.json"); + await fs.mkdir(configDir, { recursive: true }); + const logger = { warn: vi.fn(), error: vi.fn() }; + const io = createConfigIO({ + env: {} as NodeJS.ProcessEnv, + homedir: () => home, + logger, + }); + try { + await run({ + home, + configPath, + logger, + loadConfig: () => io.loadConfig() as Record, + }); + } finally { + await fs.rm(home, { recursive: true, force: true }); + } +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("config io warning/error logging", () => { + it("dedupes identical warning payloads until the config changes", async () => { + await withTempConfig(async ({ configPath, logger, loadConfig }) => { + await fs.writeFile( + configPath, + JSON.stringify( + { + plugins: { + entries: { + discord: { enabled: false, config: {} }, + }, + }, + }, + null, + 2, + ), + ); + + loadConfig(); + loadConfig(); + expect(logger.warn).toHaveBeenCalledTimes(1); + + await fs.writeFile(configPath, JSON.stringify({}, null, 2)); + loadConfig(); + + await fs.writeFile( + configPath, + JSON.stringify( + { + plugins: { + entries: { + slack: { enabled: false, config: {} }, + }, + }, + }, + null, + 2, + ), + ); + loadConfig(); + expect(logger.warn).toHaveBeenCalledTimes(2); + expect(logger.warn.mock.calls[1]?.[0]).toContain("plugins.entries.slack"); + }); + }); + + it("dedupes identical invalid config errors until the config becomes valid again", async () => { + await withTempConfig(async ({ configPath, logger, loadConfig }) => { + await fs.writeFile( + configPath, + JSON.stringify({ gateway: { port: "not-a-number" } }, null, 2), + ); + + expect(loadConfig()).toEqual({}); + expect(loadConfig()).toEqual({}); + expect(logger.error).toHaveBeenCalledTimes(1); + + await fs.writeFile(configPath, JSON.stringify({ gateway: { port: 18789 } }, null, 2)); + expect(loadConfig()).toMatchObject({ gateway: { port: 18789 } }); + + await fs.writeFile(configPath, JSON.stringify({ gateway: { port: "still-bad" } }, null, 2)); + expect(loadConfig()).toEqual({}); + expect(logger.error).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/src/config/io.ts b/src/config/io.ts index a2a2af5d1b5..88fcf81fade 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -81,7 +81,12 @@ const OPEN_DM_POLICY_ALLOW_FROM_RE = /^(?[a-z0-9_.-]+)\s*=\s*"open"\s+requires\s+(?[a-z0-9_.-]+)(?:\s+\(or\s+[a-z0-9_.-]+\))?\s+to include "\*"$/i; const CONFIG_AUDIT_LOG_FILENAME = "config-audit.jsonl"; -const loggedInvalidConfigs = new Set(); +type ConfigLogFingerprintState = { + invalid?: string; + warnings?: string; +}; + +const configLogFingerprintByPath = new Map(); type ConfigWriteAuditResult = "rename" | "copy-fallback" | "failed"; @@ -544,19 +549,63 @@ export type ConfigIoDeps = { logger?: Pick; }; -function warnOnConfigMiskeys(raw: unknown, logger: Pick): void { - if (!raw || typeof raw !== "object") { +function getConfigLogFingerprintState(configPath: string): ConfigLogFingerprintState { + let state = configLogFingerprintByPath.get(configPath); + if (!state) { + state = {}; + configLogFingerprintByPath.set(configPath, state); + } + return state; +} + +function logConfigMessageOnce(params: { + configPath: string; + kind: keyof ConfigLogFingerprintState; + message: string; + logger: Pick; +}): void { + const state = getConfigLogFingerprintState(params.configPath); + const fingerprint = hashConfigRaw(`${params.kind}:${params.message}`); + if (state[params.kind] === fingerprint) { return; } + state[params.kind] = fingerprint; + if (params.kind === "invalid") { + params.logger.error(params.message); + return; + } + params.logger.warn(params.message); +} + +function clearConfigMessageFingerprint( + configPath: string, + kind: keyof ConfigLogFingerprintState, +): void { + const state = configLogFingerprintByPath.get(configPath); + if (!state) { + return; + } + delete state[kind]; + if (!state.invalid && !state.warnings) { + configLogFingerprintByPath.delete(configPath); + } +} + +function getConfigMiskeyWarnings(raw: unknown): string[] { + const warnings: string[] = []; + if (!raw || typeof raw !== "object") { + return warnings; + } const gateway = (raw as Record).gateway; if (!gateway || typeof gateway !== "object") { - return; + return warnings; } if ("token" in (gateway as Record)) { - logger.warn( + warnings.push( 'Config uses "gateway.token". This key is ignored; use "gateway.auth.token" instead.', ); } + return warnings; } function stampConfigVersion(cfg: OpenClawConfig): OpenClawConfig { @@ -571,20 +620,19 @@ function stampConfigVersion(cfg: OpenClawConfig): OpenClawConfig { }; } -function warnIfConfigFromFuture(cfg: OpenClawConfig, logger: Pick): void { +function getFutureVersionWarning(cfg: OpenClawConfig): string | null { const touched = cfg.meta?.lastTouchedVersion; if (!touched) { - return; + return null; } const cmp = compareOpenClawVersions(VERSION, touched); if (cmp === null) { - return; + return null; } if (cmp < 0) { - logger.warn( - `Config was last written by a newer OpenClaw (${touched}); current version is ${VERSION}.`, - ); + return `Config was last written by a newer OpenClaw (${touched}); current version is ${VERSION}.`; } + return null; } function resolveConfigPathForDeps(deps: Required): string { @@ -683,6 +731,8 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { try { maybeLoadDotEnvForConfig(deps.env); if (!deps.fs.existsSync(configPath)) { + clearConfigMessageFingerprint(configPath, "invalid"); + clearConfigMessageFingerprint(configPath, "warnings"); if (shouldEnableShellEnvFallback(deps.env) && !shouldDeferShellEnvFallback(deps.env)) { loadShellEnvFallback({ enabled: true, @@ -700,8 +750,10 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { resolveConfigIncludesForRead(parsed, configPath, deps), deps.env, ); - warnOnConfigMiskeys(resolvedConfig, deps.logger); + const warningMessages = getConfigMiskeyWarnings(resolvedConfig); if (typeof resolvedConfig !== "object" || resolvedConfig === null) { + clearConfigMessageFingerprint(configPath, "invalid"); + clearConfigMessageFingerprint(configPath, "warnings"); return {}; } const preValidationDuplicates = findDuplicateAgentDirs(resolvedConfig as OpenClawConfig, { @@ -716,22 +768,39 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { const details = validated.issues .map((iss) => `- ${iss.path || ""}: ${iss.message}`) .join("\n"); - if (!loggedInvalidConfigs.has(configPath)) { - loggedInvalidConfigs.add(configPath); - deps.logger.error(`Invalid config at ${configPath}:\\n${details}`); - } + clearConfigMessageFingerprint(configPath, "warnings"); + logConfigMessageOnce({ + configPath, + kind: "invalid", + message: `Invalid config at ${configPath}:\\n${details}`, + logger: deps.logger, + }); const error = new Error(`Invalid config at ${configPath}:\n${details}`); (error as { code?: string; details?: string }).code = "INVALID_CONFIG"; (error as { code?: string; details?: string }).details = details; throw error; } + clearConfigMessageFingerprint(configPath, "invalid"); if (validated.warnings.length > 0) { const details = validated.warnings .map((iss) => `- ${iss.path || ""}: ${iss.message}`) .join("\n"); - deps.logger.warn(`Config warnings:\\n${details}`); + warningMessages.push(`Config warnings:\\n${details}`); + } + const futureVersionWarning = getFutureVersionWarning(validated.config); + if (futureVersionWarning) { + warningMessages.push(futureVersionWarning); + } + if (warningMessages.length > 0) { + logConfigMessageOnce({ + configPath, + kind: "warnings", + message: warningMessages.join("\n"), + logger: deps.logger, + }); + } else { + clearConfigMessageFingerprint(configPath, "warnings"); } - warnIfConfigFromFuture(validated.config, deps.logger); const cfg = applyTalkConfigNormalization( applyModelDefaults( applyCompactionDefaults( @@ -948,7 +1017,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { }; } - warnIfConfigFromFuture(validated.config, deps.logger); + const futureVersionWarning = getFutureVersionWarning(validated.config); const snapshotConfig = normalizeConfigPaths( applyTalkApiKey( applyTalkConfigNormalization( @@ -974,7 +1043,12 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { config: snapshotConfig, hash, issues: [], - warnings: validated.warnings, + warnings: futureVersionWarning + ? [ + ...validated.warnings, + { path: "meta.lastTouchedVersion", message: futureVersionWarning }, + ] + : validated.warnings, legacyIssues, }, envSnapshotForRestore: readResolution.envSnapshotForRestore, diff --git a/src/gateway/config-reload.test.ts b/src/gateway/config-reload.test.ts index 9c4994541e9..93c8fde20ce 100644 --- a/src/gateway/config-reload.test.ts +++ b/src/gateway/config-reload.test.ts @@ -415,4 +415,56 @@ describe("startGatewayConfigReloader", () => { await reloader.stop(); } }); + + it("dedupes unchanged invalid snapshots and re-logs when the invalid payload changes", async () => { + const readSnapshot = vi + .fn<() => Promise>() + .mockResolvedValueOnce( + makeSnapshot({ + valid: false, + issues: [ + { path: "plugins.entries.discord", message: "plugin disabled but config is present" }, + ], + hash: "invalid-1", + }), + ) + .mockResolvedValueOnce( + makeSnapshot({ + valid: false, + issues: [ + { path: "plugins.entries.discord", message: "plugin disabled but config is present" }, + ], + hash: "invalid-1", + }), + ) + .mockResolvedValueOnce( + makeSnapshot({ + valid: false, + issues: [ + { path: "plugins.entries.slack", message: "plugin disabled but config is present" }, + ], + hash: "invalid-2", + }), + ); + const { watcher, log, reloader } = createReloaderHarness(readSnapshot); + + watcher.emit("change"); + await vi.runOnlyPendingTimersAsync(); + watcher.emit("change"); + await vi.runOnlyPendingTimersAsync(); + watcher.emit("change"); + await vi.runOnlyPendingTimersAsync(); + + expect(log.warn).toHaveBeenCalledTimes(2); + expect(log.warn).toHaveBeenNthCalledWith( + 1, + "config reload skipped (invalid config): plugins.entries.discord: plugin disabled but config is present", + ); + expect(log.warn).toHaveBeenNthCalledWith( + 2, + "config reload skipped (invalid config): plugins.entries.slack: plugin disabled but config is present", + ); + + await reloader.stop(); + }); }); diff --git a/src/gateway/config-reload.ts b/src/gateway/config-reload.ts index 3887548e51b..d2a7098954a 100644 --- a/src/gateway/config-reload.ts +++ b/src/gateway/config-reload.ts @@ -89,6 +89,7 @@ export function startGatewayConfigReloader(opts: { let stopped = false; let restartQueued = false; let missingConfigRetries = 0; + let invalidSnapshotFingerprint: string | null = null; const scheduleAfter = (wait: number) => { if (stopped) { @@ -140,10 +141,15 @@ export function startGatewayConfigReloader(opts: { const handleInvalidSnapshot = (snapshot: ConfigFileSnapshot): boolean => { if (snapshot.valid) { + invalidSnapshotFingerprint = null; return false; } const issues = formatConfigIssueLines(snapshot.issues, "").join(", "); - opts.log.warn(`config reload skipped (invalid config): ${issues}`); + const fingerprint = snapshot.hash ?? issues; + if (invalidSnapshotFingerprint !== fingerprint) { + invalidSnapshotFingerprint = fingerprint; + opts.log.warn(`config reload skipped (invalid config): ${issues}`); + } return true; };