From eedd309c4a0703b1ea2ccf766da34f5bd1005418 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 6 Jul 2026 05:30:04 +0100 Subject: [PATCH] fix(config): dedupe repeated validation warnings (#100569) * fix(config): dedupe repeated validation warnings Co-authored-by: Vincent Koc * docs(changelog): defer config diagnostic note --------- Co-authored-by: Vincent Koc --- src/config/io.compat.test.ts | 67 ++++++++++++++++++++++++++++++ src/config/io.ts | 65 ++++++++++++++++++++++------- src/config/io.write-config.test.ts | 61 ++++++++++++++++++++++++++- 3 files changed, 176 insertions(+), 17 deletions(-) diff --git a/src/config/io.compat.test.ts b/src/config/io.compat.test.ts index c1e33252ee8f..ec11b9795984 100644 --- a/src/config/io.compat.test.ts +++ b/src/config/io.compat.test.ts @@ -127,6 +127,73 @@ describe("config io paths", () => { }); }); + it("logs each warning payload once until warnings clear", async () => { + await withTempHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + const logger = { + error: vi.fn(), + warn: vi.fn(), + }; + const load = () => + createConfigIO({ + configPath, + env: { HOME: home } as NodeJS.ProcessEnv, + homedir: () => home, + logger, + }).loadConfig(); + const writeRemovedPlugin = async (pluginId: string) => { + await fs.writeFile( + configPath, + JSON.stringify({ plugins: { entries: { [pluginId]: { enabled: false } } } }), + ); + }; + + await writeRemovedPlugin("google-antigravity-auth"); + load(); + load(); + expect(logger.warn).toHaveBeenCalledTimes(1); + + createConfigIO({ + configPath, + env: { HOME: home } as NodeJS.ProcessEnv, + homedir: () => home, + logger, + pluginValidation: "skip", + }).loadConfig(); + load(); + expect(logger.warn).toHaveBeenCalledTimes(1); + + await fs.writeFile( + configPath, + JSON.stringify({ + gateway: { port: "invalid" }, + plugins: { entries: { "google-antigravity-auth": { enabled: false } } }, + }), + ); + expect(load).toThrow(); + await writeRemovedPlugin("google-antigravity-auth"); + load(); + expect(logger.warn).toHaveBeenCalledTimes(1); + + await writeRemovedPlugin("google-gemini-cli-auth"); + load(); + expect(logger.warn).toHaveBeenCalledTimes(2); + + await fs.writeFile(configPath, JSON.stringify({})); + load(); + await writeRemovedPlugin("google-gemini-cli-auth"); + load(); + expect(logger.warn).toHaveBeenCalledTimes(3); + + await fs.writeFile(configPath, "null"); + load(); + await writeRemovedPlugin("google-gemini-cli-auth"); + load(); + expect(logger.warn).toHaveBeenCalledTimes(4); + }); + }); + it("explains what to check when config was written by a newer OpenClaw", async () => { await withTempHome(async (home) => { const configPath = path.join(home, ".openclaw", "openclaw.json"); diff --git a/src/config/io.ts b/src/config/io.ts index 95fa5a99e8e4..73fa35296b7d 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -167,6 +167,7 @@ type ShippedPluginInstallConfigReadMigration = { }; const loggedInvalidConfigs = new Set(); +const loggedConfigWarningFingerprints = new Map(); const warnedFutureTouchedVersions = new Set(); export type ParseConfigJson5Result = { ok: true; parsed: unknown } | { ok: false; error: string }; @@ -961,6 +962,31 @@ function warnOnConfigMiskeys(raw: unknown, logger: Pick) } } +function logConfigWarningsOnce(params: { + configPath: string; + warnings: Array<{ path: string; message: string }>; + logger: Pick; +}): void { + if (params.warnings.length === 0) { + // A later recurrence should be visible after the config becomes clean. + loggedConfigWarningFingerprints.delete(params.configPath); + return; + } + + const details = params.warnings + .map( + (warning) => + `- ${sanitizeTerminalText(warning.path || "")}: ${sanitizeTerminalText(warning.message)}`, + ) + .join("\n"); + const fingerprint = hashConfigRaw(details); + if (loggedConfigWarningFingerprints.get(params.configPath) === fingerprint) { + return; + } + loggedConfigWarningFingerprints.set(params.configPath, fingerprint); + params.logger.warn(`Config warnings:\n${details}`); +} + function stampConfigVersion(cfg: OpenClawConfig, version?: string): OpenClawConfig { return stampConfigWriteMetadata(cfg, new Date().toISOString(), version); } @@ -1687,6 +1713,7 @@ export function createConfigIO( maybeLoadDotEnvForConfig(deps.env); const envBeforeRead = snapshotEnv(deps.env); if (!deps.fs.existsSync(configPath)) { + loggedConfigWarningFingerprints.delete(configPath); if ( overrides.shellEnvFallback !== "defer" && shouldEnableShellEnvFallback(deps.env) && @@ -1726,6 +1753,7 @@ export function createConfigIO( } warnOnConfigMiskeys(validationConfigRaw, deps.logger); if (typeof validationConfigRaw !== "object" || validationConfigRaw === null) { + loggedConfigWarningFingerprints.delete(configPath); observeLoadConfigSnapshot({ ...createConfigFileSnapshot({ path: configPath, @@ -1787,14 +1815,12 @@ export function createConfigIO( loggedConfigPaths: loggedInvalidConfigs, }); } - if (validated.warnings.length > 0) { - const details = validated.warnings - .map( - (iss) => - `- ${sanitizeTerminalText(iss.path || "")}: ${sanitizeTerminalText(iss.message)}`, - ) - .join("\n"); - deps.logger.warn(`Config warnings:\n${details}`); + if (overrides.pluginValidation !== "skip") { + logConfigWarningsOnce({ + configPath, + warnings: validated.warnings, + logger: deps.logger, + }); } if (!deps.suppressFutureVersionWarning) { warnIfConfigFromFuture(validated.config, deps.logger); @@ -2396,12 +2422,7 @@ export function createConfigIO( const issueMessage = issue?.message ?? "invalid"; throw new Error(formatConfigValidationFailure(pathLabel, issueMessage)); } - if (validated.warnings.length > 0) { - const details = validated.warnings - .map((warning) => `- ${warning.path}: ${warning.message}`) - .join("\n"); - deps.logger.warn(`Config warnings:\n${details}`); - } + const previousWarningFingerprint = loggedConfigWarningFingerprints.get(configPath); // Restore ${VAR} env var references that were resolved during config loading. // Read the current file (pre-substitution) and restore any references whose @@ -2668,13 +2689,27 @@ export function createConfigIO( undefined, await deps.fs.promises.stat(configPath).catch(() => null), ); + if (!options.skipPluginValidation) { + // Only successful full-validation commits can advance warning state. + // The outer runtime refresh may still roll back this commit and state. + logConfigWarningsOnce({ + configPath, + warnings: validated.warnings, + logger: deps.logger, + }); + } return { persistedHash: nextHash, persistedConfig: stampedOutputConfig, - ...(pluginInstallConfigMigration.migrated + ...(pluginInstallConfigMigration.migrated || !options.skipPluginValidation ? { [configWritePostCommitRollback]: () => { rollbackShippedPluginInstallConfigWriteMigration(pluginInstallConfigMigration); + if (previousWarningFingerprint === undefined) { + loggedConfigWarningFingerprints.delete(configPath); + } else { + loggedConfigWarningFingerprints.set(configPath, previousWarningFingerprint); + } }, } : {}), diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index d60bb1272232..1ae0831c1adf 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -585,6 +585,46 @@ describe("config io write", () => { }); }); + it("dedupes validation warnings across writes and reloads until config becomes clean", async () => { + await withSuiteHome(async (home) => { + const warn = vi.fn(); + const io = createConfigIO({ + env: { HOME: home, OPENCLAW_TEST_FAST: "1" } as NodeJS.ProcessEnv, + homedir: () => home, + logger: { warn, error: vi.fn() }, + }); + const staleConfig = { + plugins: { entries: { demo: { enabled: true } } }, + }; + + await io.writeConfigFile(staleConfig); + await io.writeConfigFile(staleConfig); + io.loadConfig(); + expect(warn).toHaveBeenCalledTimes(1); + + await expect( + io.writeConfigFile( + {}, + { + preCommitRuntimePreflight: async () => { + throw new Error("blocked"); + }, + }, + ), + ).rejects.toThrow("blocked"); + io.loadConfig(); + expect(warn).toHaveBeenCalledTimes(1); + + await io.writeConfigFile(staleConfig, { skipPluginValidation: true }); + io.loadConfig(); + expect(warn).toHaveBeenCalledTimes(1); + + await io.writeConfigFile({}); + await io.writeConfigFile(staleConfig); + expect(warn).toHaveBeenCalledTimes(2); + }); + }); + it("keeps shipped plugin install index migration when config write fails", async () => { await withSuiteHome(async (home) => { const configPath = path.join(home, ".openclaw", "openclaw.json"); @@ -2514,9 +2554,21 @@ describe("config io write", () => { await withSuiteHome(async (home) => { const configPath = path.join(home, ".openclaw", "openclaw.json"); await fs.mkdir(path.dirname(configPath), { recursive: true }); - const initialConfig = { gateway: { mode: "local", port: 18789 } } satisfies OpenClawConfig; + const initialConfig = { + gateway: { mode: "local", port: 18789 }, + plugins: { entries: { "google-antigravity-auth": { enabled: false } } }, + } satisfies OpenClawConfig; const initialRaw = `${JSON.stringify(initialConfig, null, 2)}\n`; await fs.writeFile(configPath, initialRaw, "utf-8"); + const warn = vi.fn(); + const io = createConfigIO({ + configPath, + env: { HOME: home } as NodeJS.ProcessEnv, + homedir: () => home, + logger: { warn, error: vi.fn() }, + }); + io.loadConfig(); + expect(warn).toHaveBeenCalledTimes(1); try { await withEnvAsync({ OPENCLAW_CONFIG_PATH: configPath }, async () => { @@ -2527,10 +2579,15 @@ describe("config io write", () => { }); await expect( - writeConfigFile({ gateway: { mode: "local", port: 19001 } }), + writeConfigFile({ + gateway: { mode: "local", port: 19001 }, + plugins: { entries: { "google-gemini-cli-auth": { enabled: false } } }, + }), ).rejects.toThrow(/runtime snapshot refresh failed: synthetic refresh failure/); await expect(fs.readFile(configPath, "utf-8")).resolves.toBe(initialRaw); + io.loadConfig(); + expect(warn).toHaveBeenCalledTimes(1); }); } finally { setRuntimeConfigSnapshotRefreshHandler(null);