From 9e8d9f114d95dfdc905b06cec3146234a1f0d43d Mon Sep 17 00:00:00 2001 From: Marcus Castro Date: Sun, 8 Feb 2026 01:26:37 -0300 Subject: [PATCH] fix(cli): use raw config instead of runtime-merged config in config set/unset Fixes #6070 The config set/unset commands were using snapshot.config (which contains runtime-merged defaults) instead of snapshot.parsed (the raw user config). This caused runtime defaults like agents.defaults to leak into the written config file when any value was set or unset. Changed both set and unset commands to use structuredClone(snapshot.parsed) to preserve only user-specified config values. --- src/cli/config-cli.test.ts | 190 +++++++++++++++++++++++++++++++++++++ src/cli/config-cli.ts | 8 +- 2 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 src/cli/config-cli.test.ts diff --git a/src/cli/config-cli.test.ts b/src/cli/config-cli.test.ts new file mode 100644 index 000000000..85a109db4 --- /dev/null +++ b/src/cli/config-cli.test.ts @@ -0,0 +1,190 @@ +import { Command } from "commander"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * Test for issue #6070: + * `openclaw config set` should use snapshot.parsed (raw user config) instead of + * snapshot.config (runtime-merged config with defaults), to avoid overwriting + * the entire config with defaults when validation fails or config is unreadable. + */ + +const mockLog = vi.fn(); +const mockError = vi.fn(); +const mockExit = vi.fn((code: number) => { + const errorMessages = mockError.mock.calls.map((c) => c.join(" ")).join("; "); + throw new Error(`__exit__:${code} - ${errorMessages}`); +}); + +vi.mock("../runtime.js", () => ({ + defaultRuntime: { + log: (...args: unknown[]) => mockLog(...args), + error: (...args: unknown[]) => mockError(...args), + exit: (code: number) => mockExit(code), + }, +})); + +async function withTempHome(run: (home: string) => Promise): Promise { + const home = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-cli-")); + const originalEnv = { ...process.env }; + try { + // Override config path to use temp directory + process.env.OPENCLAW_CONFIG_PATH = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.join(home, ".openclaw"), { recursive: true }); + await run(home); + } finally { + process.env = originalEnv; + await fs.rm(home, { recursive: true, force: true }); + } +} + +async function readConfigFile(home: string): Promise> { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + const content = await fs.readFile(configPath, "utf-8"); + return JSON.parse(content); +} + +async function writeConfigFile(home: string, config: Record): Promise { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.writeFile(configPath, JSON.stringify(config, null, 2)); +} + +describe("config cli", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.resetModules(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe("config set - issue #6070", () => { + it("preserves existing config keys when setting a new value", async () => { + await withTempHome(async (home) => { + // Set up a config file with multiple existing settings (using valid schema) + const initialConfig = { + agents: { + list: [{ id: "main" }, { id: "oracle", workspace: "~/oracle-workspace" }], + }, + gateway: { + port: 18789, + }, + tools: { + allow: ["group:fs"], + }, + logging: { + level: "debug", + }, + }; + await writeConfigFile(home, initialConfig); + + // Run config set to add a new value + const { registerConfigCli } = await import("./config-cli.js"); + const program = new Command(); + program.exitOverride(); + registerConfigCli(program); + + await program.parseAsync(["config", "set", "gateway.auth.mode", "token"], { from: "user" }); + + // Read the config file and verify ALL original keys are preserved + const finalConfig = await readConfigFile(home); + + // The new value should be set + expect((finalConfig.gateway as Record).auth).toEqual({ mode: "token" }); + + // ALL original settings must still be present (this is the key assertion for #6070) + // The key bug in #6070 was that runtime defaults (like agents.defaults) were being + // written to the file, and paths were being expanded. This test verifies the fix. + expect(finalConfig.agents).not.toHaveProperty("defaults"); // No runtime defaults injected + expect((finalConfig.agents as Record).list).toEqual( + initialConfig.agents.list, + ); + expect((finalConfig.gateway as Record).port).toBe(18789); + expect(finalConfig.tools).toEqual(initialConfig.tools); + expect(finalConfig.logging).toEqual(initialConfig.logging); + }); + }); + + it("does not inject runtime defaults into the written config", async () => { + await withTempHome(async (home) => { + // Set up a minimal config file + const initialConfig = { + gateway: { port: 18789 }, + }; + await writeConfigFile(home, initialConfig); + + // Run config set + const { registerConfigCli } = await import("./config-cli.js"); + const program = new Command(); + program.exitOverride(); + registerConfigCli(program); + + await program.parseAsync(["config", "set", "gateway.auth.mode", "token"], { + from: "user", + }); + + // Read the config file + const finalConfig = await readConfigFile(home); + + // The config should NOT contain runtime defaults that weren't originally in the file + // These are examples of defaults that get merged in by applyModelDefaults, applyAgentDefaults, etc. + expect(finalConfig).not.toHaveProperty("agents.defaults.model"); + expect(finalConfig).not.toHaveProperty("agents.defaults.contextWindow"); + expect(finalConfig).not.toHaveProperty("agents.defaults.maxTokens"); + expect(finalConfig).not.toHaveProperty("messages.ackReaction"); + expect(finalConfig).not.toHaveProperty("sessions.persistence"); + + // Original config should still be present + expect((finalConfig.gateway as Record).port).toBe(18789); + // New value should be set + expect((finalConfig.gateway as Record).auth).toEqual({ mode: "token" }); + }); + }); + }); + + describe("config unset - issue #6070", () => { + it("preserves existing config keys when unsetting a value", async () => { + await withTempHome(async (home) => { + // Set up a config file with multiple existing settings (using valid schema) + const initialConfig = { + agents: { list: [{ id: "main" }] }, + gateway: { port: 18789 }, + tools: { + profile: "coding", + alsoAllow: ["agents_list"], + }, + logging: { + level: "debug", + }, + }; + await writeConfigFile(home, initialConfig); + + // Run config unset to remove a value + const { registerConfigCli } = await import("./config-cli.js"); + const program = new Command(); + program.exitOverride(); + registerConfigCli(program); + + await program.parseAsync(["config", "unset", "tools.alsoAllow"], { from: "user" }); + + // Read the config file and verify ALL original keys (except the unset one) are preserved + const finalConfig = await readConfigFile(home); + + // The value should be removed + expect(finalConfig.tools as Record).not.toHaveProperty("alsoAllow"); + + // ALL other original settings must still be present (no runtime defaults injected) + expect(finalConfig.agents).not.toHaveProperty("defaults"); + expect((finalConfig.agents as Record).list).toEqual( + initialConfig.agents.list, + ); + expect(finalConfig.gateway).toEqual(initialConfig.gateway); + expect((finalConfig.tools as Record).profile).toBe("coding"); + expect(finalConfig.logging).toEqual(initialConfig.logging); + }); + }); + }); +}); diff --git a/src/cli/config-cli.ts b/src/cli/config-cli.ts index 7eabdef99..9b1276fd0 100644 --- a/src/cli/config-cli.ts +++ b/src/cli/config-cli.ts @@ -306,7 +306,9 @@ export function registerConfigCli(program: Command) { } const parsedValue = parseValue(value, opts); const snapshot = await loadValidConfig(); - const next = snapshot.config as Record; + // Use snapshot.parsed (raw user config) instead of snapshot.config (runtime-merged with defaults) + // This prevents runtime defaults from leaking into the written config file (issue #6070) + const next = structuredClone(snapshot.parsed) as Record; setAtPath(next, parsedPath, parsedValue); await writeConfigFile(next); defaultRuntime.log(info(`Updated ${path}. Restart the gateway to apply.`)); @@ -327,7 +329,9 @@ export function registerConfigCli(program: Command) { throw new Error("Path is empty."); } const snapshot = await loadValidConfig(); - const next = snapshot.config as Record; + // Use snapshot.parsed (raw user config) instead of snapshot.config (runtime-merged with defaults) + // This prevents runtime defaults from leaking into the written config file (issue #6070) + const next = structuredClone(snapshot.parsed) as Record; const removed = unsetAtPath(next, parsedPath); if (!removed) { defaultRuntime.error(danger(`Config path not found: ${path}`));