mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix(config): dedupe repeated validation warnings (#100569)
* fix(config): dedupe repeated validation warnings Co-authored-by: Vincent Koc <vincentkoc@ieee.org> * docs(changelog): defer config diagnostic note --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
committed by
GitHub
parent
2b51c255f8
commit
eedd309c4a
@@ -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");
|
||||
|
||||
+50
-15
@@ -167,6 +167,7 @@ type ShippedPluginInstallConfigReadMigration = {
|
||||
};
|
||||
|
||||
const loggedInvalidConfigs = new Set<string>();
|
||||
const loggedConfigWarningFingerprints = new Map<string, string>();
|
||||
const warnedFutureTouchedVersions = new Set<string>();
|
||||
|
||||
export type ParseConfigJson5Result = { ok: true; parsed: unknown } | { ok: false; error: string };
|
||||
@@ -961,6 +962,31 @@ function warnOnConfigMiskeys(raw: unknown, logger: Pick<typeof console, "warn">)
|
||||
}
|
||||
}
|
||||
|
||||
function logConfigWarningsOnce(params: {
|
||||
configPath: string;
|
||||
warnings: Array<{ path: string; message: string }>;
|
||||
logger: Pick<typeof console, "warn">;
|
||||
}): 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 || "<root>")}: ${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 || "<root>")}: ${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);
|
||||
}
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user