diff --git a/CHANGELOG.md b/CHANGELOG.md index 6004271cab9..1c4a8e6f205 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,7 @@ Docs: https://docs.openclaw.ai - Cron/announce: preserve all deliverable text payloads for announce mode instead of collapsing to the last chunk, so multi-line cron reports deliver in full to Telegram forum topics. - Harden async approval followup delivery in webchat-only sessions (#57359) Thanks @joshavant. - Status: fix cache hit rate exceeding 100% by deriving denominator from prompt-side token fields instead of potentially undersized totalTokens. Fixes #26643. +- Config/update: stop `openclaw doctor` write-backs from persisting plugin-injected channel defaults, so `openclaw update` no longer seeds config keys that later break service refresh validation. (#56834) Thanks @openperf. ## 2026.3.28 diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index 58c63fd288b..7e641b03f30 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -638,12 +638,14 @@ async function maybeRestartService(params: { invocationCwd: params.invocationCwd, }); } catch (err) { - if (!params.opts.json) { - defaultRuntime.log( - theme.warn( - `Failed to refresh gateway service environment from updated install: ${String(err)}`, - ), - ); + // Always log the refresh failure so callers can detect it (issue #56772). + // Previously this was silently suppressed in --json mode, hiding the root + // cause and preventing auto-update callers from detecting the failure. + const message = `Failed to refresh gateway service environment from updated install: ${String(err)}`; + if (params.opts.json) { + defaultRuntime.error(message); + } else { + defaultRuntime.log(theme.warn(message)); } } } diff --git a/src/config/io.ts b/src/config/io.ts index 65e8d064965..9d586c50e5f 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -31,6 +31,7 @@ import { resolveConfigIncludes, } from "./includes.js"; import { migrateLegacyConfig } from "./legacy-migrate.js"; +import { normalizeLegacyWebSearchConfig } from "./legacy-web-search.js"; import { findLegacyConfigIssues } from "./legacy.js"; import { asResolvedSourceConfig, @@ -2127,9 +2128,13 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) { // // We use only the root file's parsed content (no $include resolution) to avoid // pulling values from included files into the root config on write-back. - // Apply env restoration to validated.config (which has runtime defaults stripped - // per issue #6070) rather than the raw caller input. - let cfgToWrite = validated.config; + // Use persistCandidate (the merge-patched value before validation) rather than + // validated.config, because plugin/channel AJV validation may inject schema + // defaults (e.g., enrichGroupParticipantsFromContacts) that should not be + // persisted to disk (issue #56772). + // Apply legacy web-search normalization so that migration results are still + // persisted even though we bypass validated.config. + let cfgToWrite = normalizeLegacyWebSearchConfig(persistCandidate) as OpenClawConfig; try { if (deps.fs.existsSync(configPath)) { const currentRaw = await deps.fs.promises.readFile(configPath, "utf-8"); diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index f0f92f10c78..56d6de63078 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -5,6 +5,16 @@ import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { createConfigIO } from "./io.js"; import type { OpenClawConfig } from "./types.js"; +// Mock the plugin manifest registry so we can register a fake channel whose +// AJV JSON Schema carries a `default` value. This lets the #56772 regression +// test exercise the exact code path that caused the bug: AJV injecting +// defaults during the write-back validation pass. +const mockLoadPluginManifestRegistry = vi.hoisted(() => vi.fn()); + +vi.mock("../plugins/manifest-registry.js", () => ({ + loadPluginManifestRegistry: (...args: unknown[]) => mockLoadPluginManifestRegistry(...args), +})); + describe("config io write", () => { let fixtureRoot = ""; let homeCaseId = 0; @@ -21,6 +31,13 @@ describe("config io write", () => { beforeAll(async () => { fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-io-")); + + // Default: return an empty plugin list so existing tests that don't need + // plugin-owned channel schemas keep working unchanged. + mockLoadPluginManifestRegistry.mockReturnValue({ + diagnostics: [], + plugins: [], + }); }); afterAll(async () => { @@ -353,6 +370,95 @@ describe("config io write", () => { }); }); + it("does not leak channel plugin AJV defaults into persisted config (issue #56772)", async () => { + // Regression test for #56772. Mock the BlueBubbles channel metadata so + // read-time AJV validation injects the same default that triggered the + // write-back leak. + mockLoadPluginManifestRegistry.mockReturnValue({ + diagnostics: [], + plugins: [ + { + id: "bluebubbles", + origin: "bundled", + channels: ["bluebubbles"], + channelCatalogMeta: { + id: "bluebubbles", + label: "BlueBubbles", + blurb: "BlueBubbles channel", + }, + channelConfigs: { + bluebubbles: { + schema: { + type: "object", + properties: { + enrichGroupParticipantsFromContacts: { + type: "boolean", + default: true, + }, + serverUrl: { + type: "string", + }, + }, + additionalProperties: true, + }, + uiHints: {}, + }, + }, + }, + ], + }); + + await withSuiteHome(async (home) => { + const { configPath, io, snapshot } = await writeConfigAndCreateIo({ + home, + initialConfig: { + gateway: { port: 18789 }, + channels: { + bluebubbles: { + serverUrl: "http://localhost:1234", + }, + }, + }, + }); + + // Simulate doctor: clone snapshot.config, make a small change, write back. + const next = structuredClone(snapshot.config); + const gateway = + next.gateway && typeof next.gateway === "object" + ? (next.gateway as Record) + : {}; + next.gateway = { + ...gateway, + auth: { mode: "token" }, + }; + await io.writeConfigFile(next); + + const persisted = JSON.parse(await fs.readFile(configPath, "utf-8")) as Record< + string, + unknown + >; + + // The persisted config should contain only explicitly set values. + expect(persisted.gateway).toEqual({ + port: 18789, + auth: { mode: "token" }, + }); + + // The critical assertion: the AJV-injected BlueBubbles default must not + // appear in the persisted config. + const channels = persisted.channels as Record> | undefined; + expect(channels?.bluebubbles).toBeDefined(); + expect(channels?.bluebubbles).not.toHaveProperty("enrichGroupParticipantsFromContacts"); + expect(channels?.bluebubbles?.serverUrl).toBe("http://localhost:1234"); + }); + + // Restore the default empty-plugins mock for subsequent tests. + mockLoadPluginManifestRegistry.mockReturnValue({ + diagnostics: [], + plugins: [], + }); + }); + it("does not reintroduce Slack/Discord legacy dm.policy defaults when writing", async () => { await withSuiteHome(async (home) => { const { configPath, io, snapshot } = await writeConfigAndCreateIo({ diff --git a/src/config/validation.channel-metadata.test.ts b/src/config/validation.channel-metadata.test.ts index fae1d88b9fc..3961a35e4a7 100644 --- a/src/config/validation.channel-metadata.test.ts +++ b/src/config/validation.channel-metadata.test.ts @@ -6,42 +6,46 @@ vi.mock("../plugins/manifest-registry.js", () => ({ loadPluginManifestRegistry: (...args: unknown[]) => mockLoadPluginManifestRegistry(...args), })); -describe("validateConfigObjectRawWithPlugins channel metadata", () => { - it("applies bundled channel defaults from plugin-owned schema metadata", async () => { - mockLoadPluginManifestRegistry.mockReturnValue({ - diagnostics: [], - plugins: [ - { +function setupTelegramSchemaWithDefault() { + mockLoadPluginManifestRegistry.mockReturnValue({ + diagnostics: [], + plugins: [ + { + id: "telegram", + origin: "bundled", + channels: ["telegram"], + channelCatalogMeta: { id: "telegram", - origin: "bundled", - channels: ["telegram"], - channelCatalogMeta: { - id: "telegram", - label: "Telegram", - blurb: "Telegram channel", - }, - channelConfigs: { - telegram: { - schema: { - type: "object", - properties: { - dmPolicy: { - type: "string", - enum: ["pairing", "allowlist"], - default: "pairing", - }, + label: "Telegram", + blurb: "Telegram channel", + }, + channelConfigs: { + telegram: { + schema: { + type: "object", + properties: { + dmPolicy: { + type: "string", + enum: ["pairing", "allowlist"], + default: "pairing", }, - additionalProperties: false, }, - uiHints: {}, + additionalProperties: false, }, + uiHints: {}, }, }, - ], - }); + }, + ], + }); +} - const { validateConfigObjectRawWithPlugins } = await import("./validation.js"); - const result = validateConfigObjectRawWithPlugins({ +describe("validateConfigObjectWithPlugins channel metadata (applyDefaults: true)", () => { + it("applies bundled channel defaults from plugin-owned schema metadata", async () => { + setupTelegramSchemaWithDefault(); + + const { validateConfigObjectWithPlugins } = await import("./validation.js"); + const result = validateConfigObjectWithPlugins({ channels: { telegram: {}, }, @@ -55,3 +59,32 @@ describe("validateConfigObjectRawWithPlugins channel metadata", () => { } }); }); + +describe("validateConfigObjectRawWithPlugins channel metadata", () => { + it("still injects channel AJV defaults even in raw mode — persistence safety is handled by io.ts", async () => { + // Channel and plugin AJV validation always runs with applyDefaults: true + // (hardcoded) to avoid breaking schemas that mark defaulted fields as + // required (e.g., BlueBubbles enrichGroupParticipantsFromContacts). + // + // The actual protection against leaking these defaults to disk lives in + // writeConfigFile (io.ts), which uses persistCandidate (the pre-validation + // merge-patched value) instead of validated.config. + setupTelegramSchemaWithDefault(); + + const { validateConfigObjectRawWithPlugins } = await import("./validation.js"); + const result = validateConfigObjectRawWithPlugins({ + channels: { + telegram: {}, + }, + }); + + expect(result.ok).toBe(true); + if (result.ok) { + // AJV defaults ARE injected into validated.config even in raw mode. + // This is intentional — see comment above. + expect(result.config.channels?.telegram).toEqual( + expect.objectContaining({ dmPolicy: "pairing" }), + ); + } + }); +});