Skip to content

Commit a27ccee

Browse files
steipetesteipete
committed
refactor(config): use source snapshots for config writes
1 parent c5baf63 commit a27ccee

18 files changed

Lines changed: 154 additions & 72 deletions

‎src/cli/channel-auth.test.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ const mocks = vi.hoisted(() => ({
1111
listChannelPlugins: vi.fn(),
1212
normalizeChannelId: vi.fn(),
1313
loadConfig: vi.fn(),
14+
readConfigFileSnapshot: vi.fn(),
1415
applyPluginAutoEnable: vi.fn(),
15-
writeConfigFile: vi.fn(),
16+
replaceConfigFile: vi.fn(),
1617
setVerbose: vi.fn(),
1718
createClackPrompter: vi.fn(),
1819
ensureChannelSetupPluginInstalled: vi.fn(),
@@ -44,7 +45,8 @@ vi.mock("../channels/plugins/index.js", () => ({
4445

4546
vi.mock("../config/config.js", () => ({
4647
loadConfig: mocks.loadConfig,
47-
writeConfigFile: mocks.writeConfigFile,
48+
readConfigFileSnapshot: mocks.readConfigFileSnapshot,
49+
replaceConfigFile: mocks.replaceConfigFile,
4850
}));
4951

5052
vi.mock("../config/plugin-auto-enable.js", () => ({
@@ -84,8 +86,9 @@ describe("channel-auth", () => {
8486
mocks.getChannelPluginCatalogEntry.mockReturnValue(undefined);
8587
mocks.listChannelPluginCatalogEntries.mockReturnValue([]);
8688
mocks.loadConfig.mockReturnValue({ channels: { whatsapp: {} } });
89+
mocks.readConfigFileSnapshot.mockResolvedValue({ hash: "config-1" });
8790
mocks.applyPluginAutoEnable.mockImplementation(({ config }) => ({ config, changes: [] }));
88-
mocks.writeConfigFile.mockResolvedValue(undefined);
91+
mocks.replaceConfigFile.mockResolvedValue(undefined);
8992
mocks.listChannelPlugins.mockReturnValue([plugin]);
9093
mocks.resolveDefaultAgentId.mockReturnValue("main");
9194
mocks.resolveAgentWorkspaceDir.mockReturnValue("/tmp/workspace");
@@ -158,7 +161,10 @@ describe("channel-auth", () => {
158161
channelInput: "whatsapp",
159162
}),
160163
);
161-
expect(mocks.writeConfigFile).toHaveBeenCalledWith(autoEnabledCfg);
164+
expect(mocks.replaceConfigFile).toHaveBeenCalledWith({
165+
nextConfig: autoEnabledCfg,
166+
baseHash: "config-1",
167+
});
162168
});
163169

164170
it("persists auto-enabled config during logout auto-pick too", async () => {
@@ -173,7 +179,10 @@ describe("channel-auth", () => {
173179
cfg: autoEnabledCfg,
174180
}),
175181
);
176-
expect(mocks.writeConfigFile).toHaveBeenCalledWith(autoEnabledCfg);
182+
expect(mocks.replaceConfigFile).toHaveBeenCalledWith({
183+
nextConfig: autoEnabledCfg,
184+
baseHash: "config-1",
185+
});
177186
});
178187

179188
it("ignores configured channels that do not support login when channel is omitted", async () => {
@@ -304,7 +313,10 @@ describe("channel-auth", () => {
304313
workspaceDir: "/tmp/workspace",
305314
}),
306315
);
307-
expect(mocks.writeConfigFile).toHaveBeenCalledWith({ channels: { whatsapp: {} } });
316+
expect(mocks.replaceConfigFile).toHaveBeenCalledWith({
317+
nextConfig: { channels: { whatsapp: {} } },
318+
baseHash: "config-1",
319+
});
308320
expect(mocks.login).toHaveBeenCalled();
309321
});
310322

‎src/cli/channel-auth.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@ import {
55
normalizeChannelId,
66
} from "../channels/plugins/index.js";
77
import { resolveInstallableChannelPlugin } from "../commands/channel-setup/channel-plugin-resolution.js";
8-
import { loadConfig, writeConfigFile, type OpenClawConfig } from "../config/config.js";
8+
import {
9+
loadConfig,
10+
readConfigFileSnapshot,
11+
replaceConfigFile,
12+
type OpenClawConfig,
13+
} from "../config/config.js";
914
import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js";
1015
import { setVerbose } from "../globals.js";
1116
import { isBlockedObjectKey } from "../infra/prototype-keys.js";
@@ -131,6 +136,7 @@ export async function runChannelLogin(
131136
opts: ChannelAuthOptions,
132137
runtime: RuntimeEnv = defaultRuntime,
133138
) {
139+
const sourceSnapshotPromise = readConfigFileSnapshot().catch(() => null);
134140
const autoEnabled = applyPluginAutoEnable({
135141
config: loadConfig(),
136142
env: process.env,
@@ -143,7 +149,10 @@ export async function runChannelLogin(
143149
runtime,
144150
);
145151
if (autoEnabled.changes.length > 0 || configChanged) {
146-
await writeConfigFile(cfg);
152+
await replaceConfigFile({
153+
nextConfig: cfg,
154+
baseHash: (await sourceSnapshotPromise)?.hash,
155+
});
147156
}
148157
const login = plugin.auth?.login;
149158
if (!login) {
@@ -165,6 +174,7 @@ export async function runChannelLogout(
165174
opts: ChannelAuthOptions,
166175
runtime: RuntimeEnv = defaultRuntime,
167176
) {
177+
const sourceSnapshotPromise = readConfigFileSnapshot().catch(() => null);
168178
const autoEnabled = applyPluginAutoEnable({
169179
config: loadConfig(),
170180
env: process.env,
@@ -177,7 +187,10 @@ export async function runChannelLogout(
177187
runtime,
178188
);
179189
if (autoEnabled.changes.length > 0 || configChanged) {
180-
await writeConfigFile(cfg);
190+
await replaceConfigFile({
191+
nextConfig: cfg,
192+
baseHash: (await sourceSnapshotPromise)?.hash,
193+
});
181194
}
182195
const logoutAccount = plugin.gateway?.logoutAccount;
183196
if (!logoutAccount) {

‎src/cli/directory-cli.test.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ function getRuntimeCapture(): CliRuntimeCapture {
1414

1515
const mocks = vi.hoisted(() => ({
1616
loadConfig: vi.fn(),
17+
readConfigFileSnapshot: vi.fn(),
1718
applyPluginAutoEnable: vi.fn(),
18-
writeConfigFile: vi.fn(),
19+
replaceConfigFile: vi.fn(),
1920
resolveInstallableChannelPlugin: vi.fn(),
2021
resolveMessageChannelSelection: vi.fn(),
2122
getChannelPlugin: vi.fn(),
@@ -24,7 +25,8 @@ const mocks = vi.hoisted(() => ({
2425

2526
vi.mock("../config/config.js", () => ({
2627
loadConfig: mocks.loadConfig,
27-
writeConfigFile: mocks.writeConfigFile,
28+
readConfigFileSnapshot: mocks.readConfigFileSnapshot,
29+
replaceConfigFile: mocks.replaceConfigFile,
2830
}));
2931

3032
vi.mock("../config/plugin-auto-enable.js", () => ({
@@ -58,8 +60,9 @@ describe("registerDirectoryCli", () => {
5860
vi.clearAllMocks();
5961
getRuntimeCapture().resetRuntimeCapture();
6062
mocks.loadConfig.mockReturnValue({ channels: {} });
63+
mocks.readConfigFileSnapshot.mockResolvedValue({ hash: "config-1" });
6164
mocks.applyPluginAutoEnable.mockImplementation(({ config }) => ({ config, changes: [] }));
62-
mocks.writeConfigFile.mockResolvedValue(undefined);
65+
mocks.replaceConfigFile.mockResolvedValue(undefined);
6366
mocks.resolveChannelDefaultAccountId.mockReturnValue("default");
6467
mocks.resolveMessageChannelSelection.mockResolvedValue({
6568
channel: "demo-channel",
@@ -99,11 +102,12 @@ describe("registerDirectoryCli", () => {
99102
allowInstall: true,
100103
}),
101104
);
102-
expect(mocks.writeConfigFile).toHaveBeenCalledWith(
103-
expect.objectContaining({
105+
expect(mocks.replaceConfigFile).toHaveBeenCalledWith({
106+
nextConfig: expect.objectContaining({
104107
plugins: { entries: { "demo-directory": { enabled: true } } },
105108
}),
106-
);
109+
baseHash: "config-1",
110+
});
107111
expect(self).toHaveBeenCalledWith(
108112
expect.objectContaining({
109113
accountId: "default",
@@ -150,6 +154,9 @@ describe("registerDirectoryCli", () => {
150154
cfg: autoEnabledConfig,
151155
}),
152156
);
153-
expect(mocks.writeConfigFile).toHaveBeenCalledWith(autoEnabledConfig);
157+
expect(mocks.replaceConfigFile).toHaveBeenCalledWith({
158+
nextConfig: autoEnabledConfig,
159+
baseHash: "config-1",
160+
});
154161
});
155162
});

‎src/cli/directory-cli.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { Command } from "commander";
22
import { resolveChannelDefaultAccountId } from "../channels/plugins/helpers.js";
33
import { getChannelPlugin } from "../channels/plugins/index.js";
44
import { resolveInstallableChannelPlugin } from "../commands/channel-setup/channel-plugin-resolution.js";
5-
import { loadConfig, writeConfigFile } from "../config/config.js";
5+
import { loadConfig, readConfigFileSnapshot, replaceConfigFile } from "../config/config.js";
66
import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js";
77
import { danger } from "../globals.js";
88
import { resolveMessageChannelSelection } from "../infra/outbound/channel-selection.js";
@@ -98,6 +98,7 @@ export function registerDirectoryCli(program: Command) {
9898
.option("--json", "Output JSON", false);
9999

100100
const resolve = async (opts: { channel?: string; account?: string }) => {
101+
const sourceSnapshotPromise = readConfigFileSnapshot().catch(() => null);
101102
const autoEnabled = applyPluginAutoEnable({
102103
config: loadConfig(),
103104
env: process.env,
@@ -115,9 +116,15 @@ export function registerDirectoryCli(program: Command) {
115116
: null;
116117
if (resolvedExplicit?.configChanged) {
117118
cfg = resolvedExplicit.cfg;
118-
await writeConfigFile(cfg);
119+
await replaceConfigFile({
120+
nextConfig: cfg,
121+
baseHash: (await sourceSnapshotPromise)?.hash,
122+
});
119123
} else if (autoEnabled.changes.length > 0) {
120-
await writeConfigFile(cfg);
124+
await replaceConfigFile({
125+
nextConfig: cfg,
126+
baseHash: (await sourceSnapshotPromise)?.hash,
127+
});
121128
}
122129
const selection = explicitChannel
123130
? {

‎src/commands/configure.wizard.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,9 @@ export async function runConfigureWizard(
237237
const prompter = createClackPrompter();
238238

239239
const snapshot = await readConfigFileSnapshot();
240-
const baseConfig: OpenClawConfig = snapshot.valid ? snapshot.config : {};
240+
const baseConfig: OpenClawConfig = snapshot.valid
241+
? (snapshot.sourceConfig ?? snapshot.config)
242+
: {};
241243

242244
if (snapshot.exists) {
243245
const title = snapshot.valid ? "Existing config detected" : "Invalid config";

‎src/commands/dashboard.links.test.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
22

33
const readConfigFileSnapshotMock = vi.hoisted(() => vi.fn());
44
const resolveGatewayPortMock = vi.hoisted(() => vi.fn());
@@ -63,11 +63,9 @@ function mockSnapshot(token: unknown = "abc") {
6363
}
6464

6565
describe("dashboardCommand", () => {
66-
beforeAll(async () => {
66+
beforeEach(async () => {
67+
vi.resetModules();
6768
({ dashboardCommand } = await import("./dashboard.js"));
68-
});
69-
70-
beforeEach(() => {
7169
resetRuntime();
7270
readConfigFileSnapshotMock.mockClear();
7371
resolveGatewayPortMock.mockClear();

‎src/commands/dashboard.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { beforeEach, describe, expect, it, vi } from "vitest";
22
import type { GatewayBindMode } from "../config/types.gateway.js";
3-
import { dashboardCommand } from "./dashboard.js";
43

54
const mocks = vi.hoisted(() => ({
65
readConfigFileSnapshot: vi.fn(),
@@ -30,6 +29,7 @@ const runtime = {
3029
error: vi.fn(),
3130
exit: vi.fn(),
3231
};
32+
let dashboardCommand: typeof import("./dashboard.js").dashboardCommand;
3333

3434
function mockSnapshot(params?: {
3535
token?: string;
@@ -62,7 +62,9 @@ function mockSnapshot(params?: {
6262
}
6363

6464
describe("dashboardCommand bind selection", () => {
65-
beforeEach(() => {
65+
beforeEach(async () => {
66+
vi.resetModules();
67+
({ dashboardCommand } = await import("./dashboard.js"));
6668
mocks.readConfigFileSnapshot.mockClear();
6769
mocks.resolveGatewayPort.mockClear();
6870
mocks.resolveControlUiLinks.mockClear();

‎src/commands/dashboard.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export async function dashboardCommand(
5252
options: DashboardOptions = {},
5353
) {
5454
const snapshot = await readConfigFileSnapshot();
55-
const cfg = snapshot.valid ? snapshot.config : {};
55+
const cfg = snapshot.valid ? (snapshot.sourceConfig ?? snapshot.config) : {};
5656
const port = resolveGatewayPort(cfg);
5757
const bind = cfg.gateway?.bind ?? "loopback";
5858
const basePath = cfg.gateway?.controlUi?.basePath;

‎src/commands/doctor-config-preflight.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,6 @@ export async function runDoctorConfigPreflight(
100100

101101
return {
102102
snapshot,
103-
baseConfig: snapshot.config ?? {},
103+
baseConfig: snapshot.sourceConfig ?? snapshot.config ?? {},
104104
};
105105
}

‎src/commands/gateway-install-token.test.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
22
import type { OpenClawConfig } from "../config/config.js";
33

44
const readConfigFileSnapshotMock = vi.hoisted(() => vi.fn());
5-
const writeConfigFileMock = vi.hoisted(() => vi.fn());
5+
const replaceConfigFileMock = vi.hoisted(() => vi.fn());
66
const resolveSecretInputRefMock = vi.hoisted(() =>
77
vi.fn((): { ref: unknown } => ({ ref: undefined })),
88
);
@@ -29,7 +29,7 @@ const randomTokenMock = vi.hoisted(() => vi.fn(() => "generated-token"));
2929

3030
vi.mock("../config/config.js", () => ({
3131
readConfigFileSnapshot: readConfigFileSnapshotMock,
32-
writeConfigFile: writeConfigFileMock,
32+
replaceConfigFile: replaceConfigFileMock,
3333
}));
3434

3535
vi.mock("../config/types.secrets.js", () => ({
@@ -153,7 +153,7 @@ describe("resolveGatewayInstallToken", () => {
153153
expect(result.unavailableReason).toContain("gateway.auth.mode is unset");
154154
expect(result.unavailableReason).toContain("openclaw config set gateway.auth.mode token");
155155
expect(result.unavailableReason).toContain("openclaw config set gateway.auth.mode password");
156-
expect(writeConfigFileMock).not.toHaveBeenCalled();
156+
expect(replaceConfigFileMock).not.toHaveBeenCalled();
157157
expect(resolveSecretRefValuesMock).not.toHaveBeenCalled();
158158
});
159159

@@ -171,7 +171,7 @@ describe("resolveGatewayInstallToken", () => {
171171
expect(
172172
result.warnings.some((message) => message.includes("without saving to config")),
173173
).toBeTruthy();
174-
expect(writeConfigFileMock).not.toHaveBeenCalled();
174+
expect(replaceConfigFileMock).not.toHaveBeenCalled();
175175
});
176176

177177
it("persists auto-generated token when requested", async () => {
@@ -185,16 +185,17 @@ describe("resolveGatewayInstallToken", () => {
185185
});
186186

187187
expect(result.warnings.some((message) => message.includes("saving to config"))).toBeTruthy();
188-
expect(writeConfigFileMock).toHaveBeenCalledWith(
189-
expect.objectContaining({
188+
expect(replaceConfigFileMock).toHaveBeenCalledWith({
189+
baseHash: undefined,
190+
nextConfig: expect.objectContaining({
190191
gateway: {
191192
auth: {
192193
mode: "token",
193194
token: "generated-token",
194195
},
195196
},
196197
}),
197-
);
198+
});
198199
});
199200

200201
it("drops generated plaintext when config changes to SecretRef before persist", async () => {
@@ -227,7 +228,7 @@ describe("resolveGatewayInstallToken", () => {
227228
expect(
228229
result.warnings.some((message) => message.includes("skipping plaintext token persistence")),
229230
).toBeTruthy();
230-
expect(writeConfigFileMock).not.toHaveBeenCalled();
231+
expect(replaceConfigFileMock).not.toHaveBeenCalled();
231232
});
232233

233234
it("does not auto-generate when inferred mode has password SecretRef configured", async () => {
@@ -254,7 +255,7 @@ describe("resolveGatewayInstallToken", () => {
254255
expect(result.token).toBeUndefined();
255256
expect(result.unavailableReason).toBeUndefined();
256257
expect(result.warnings.some((message) => message.includes("Auto-generated"))).toBe(false);
257-
expect(writeConfigFileMock).not.toHaveBeenCalled();
258+
expect(replaceConfigFileMock).not.toHaveBeenCalled();
258259
});
259260

260261
it("passes the install env through to gateway auth resolution", async () => {
@@ -286,7 +287,7 @@ describe("resolveGatewayInstallToken", () => {
286287
expect(result.token).toBeUndefined();
287288
expect(result.unavailableReason).toBeUndefined();
288289
expect(result.warnings.some((message) => message.includes("Auto-generated"))).toBe(false);
289-
expect(writeConfigFileMock).not.toHaveBeenCalled();
290+
expect(replaceConfigFileMock).not.toHaveBeenCalled();
290291
});
291292

292293
it("skips token SecretRef resolution when token auth is not required", async () => {

0 commit comments

Comments
 (0)