From 804bb0f2c30ec2e997e0b9d707ddb332f02a2ceb Mon Sep 17 00:00:00 2001 From: Pengfei Ni Date: Wed, 15 Apr 2026 18:03:09 +0800 Subject: [PATCH] fix(configure): re-read config hash after persist to avoid stale-hash race (#64188) (#66528) Merged via squash. Prepared head SHA: 0c4003a5befa86ed967886f747b0c381fac1eaba Co-authored-by: feiskyer <676637+feiskyer@users.noreply.github.com> Co-authored-by: vincentkoc <25068+vincentkoc@users.noreply.github.com> Reviewed-by: @vincentkoc --- CHANGELOG.md | 1 + src/commands/configure.wizard.test.ts | 128 ++++++++++++++++++++++++++ src/commands/configure.wizard.ts | 68 ++++++++++++-- 3 files changed, 190 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d6afc6facf..988b97f1972 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -212,6 +212,7 @@ Docs: https://docs.openclaw.ai - Cron/direct delivery: slim isolated-agent delivery cold paths so direct channel delivery and related cron execution spend less time loading unrelated auth, plugin, and channel runtime. Thanks @vincentkoc. - Channels/replay dedupe: standardize replay claims, retryable-failure release, and post-success commit behavior across Telegram, Discord, Slack, Mattermost, WhatsApp, Matrix, LINE, Feishu, Zalo, Nextcloud Talk, TLON, Nostr, Voice Call, and shared plugin interactive callbacks so duplicate deliveries stay reply-once after success but retry cleanly after pre-delivery failures. Thanks @vincentkoc. - Agents/OpenAI mini reasoning: remap unsupported `low` and `minimal` reasoning effort to `medium` for affected OpenAI mini models, and add a live regression lane to keep the compatibility fix covered. (#65478) Thanks @vincentkoc. +- Configure/wizard: replay wizard edits onto the latest config snapshot after a hash conflict so plugin-auth writes no longer get dropped during `openclaw configure`, including nested config under shared sections such as `plugins`. (#64188) Thanks @feiskyer and @vincentkoc. ## 2026.4.11 diff --git a/src/commands/configure.wizard.test.ts b/src/commands/configure.wizard.test.ts index a078cf34136..b3266c133d1 100644 --- a/src/commands/configure.wizard.test.ts +++ b/src/commands/configure.wizard.test.ts @@ -109,6 +109,15 @@ vi.mock("./onboard-search.js", () => ({ setupSearch: mocks.setupSearch, })); +vi.mock("../config/mutate.js", async () => { + const actual = await vi.importActual("../config/mutate.js"); + return { + ...actual, + ConfigMutationConflictError: actual.ConfigMutationConflictError, + }; +}); + +import { ConfigMutationConflictError } from "../config/mutate.js"; import { WizardCancelledError } from "../wizard/prompts.js"; import { runConfigureWizard } from "./configure.wizard.js"; @@ -451,4 +460,123 @@ describe("runConfigureWizard", () => { ); expect(mocks.setupSearch).toHaveBeenCalledOnce(); }); + + it("retries without dropping nested plugin config written during wizard flow (issue #64188)", async () => { + const baseConfig: OpenClawConfig = { + plugins: { + entries: { + "github-copilot": { + enabled: false, + config: { + region: "us-east-1", + }, + }, + }, + }, + }; + setupBaseWizardState(baseConfig); + queueWizardPrompts({ + select: ["local"], + confirm: [], + }); + + // Simulate plugin mutation: first replaceConfigFile call throws conflict, + // second call after hash refresh succeeds + let callCount = 0; + const originalHash = "hash-before-plugin-mutation"; + const newHashAfterMutation = "hash-after-plugin-mutation"; + const finalHashAfterWrite = "hash-after-wizard-write"; + + mocks.replaceConfigFile.mockImplementation( + async (params: { nextConfig: unknown; baseHash?: string }) => { + callCount++; + if (callCount === 1) { + // First call: simulate plugin mutating config during promptAuthConfig + expect(params.baseHash).toBe(originalHash); + throw new ConfigMutationConflictError("config changed since last load", { + currentHash: newHashAfterMutation, + }); + } + // Second call: succeeds with refreshed hash + expect(params.baseHash).toBe(newHashAfterMutation); + await mocks.writeConfigFile(params.nextConfig); + }, + ); + + // Mock readConfigFileSnapshot to return different hashes/configs on each call + mocks.readConfigFileSnapshot + .mockResolvedValueOnce({ + ...EMPTY_CONFIG_SNAPSHOT, + hash: originalHash, + config: baseConfig, + sourceConfig: baseConfig, + }) + .mockResolvedValueOnce({ + ...EMPTY_CONFIG_SNAPSHOT, + hash: newHashAfterMutation, + config: { + plugins: { + entries: { + "github-copilot": { + enabled: false, + config: { + region: "us-east-1", + accessToken: "plugin-wrote-this", + }, + }, + }, + }, + }, + sourceConfig: { + plugins: { + entries: { + "github-copilot": { + enabled: false, + config: { + region: "us-east-1", + accessToken: "plugin-wrote-this", + }, + }, + }, + }, + }, + valid: true, + }) + .mockResolvedValueOnce({ + ...EMPTY_CONFIG_SNAPSHOT, + hash: finalHashAfterWrite, + config: {}, + }); + + await runConfigureWizard({ command: "configure", sections: ["workspace"] }, createRuntime()); + + // Verify retry happened: first call threw, second call succeeded + expect(mocks.replaceConfigFile).toHaveBeenCalledTimes(2); + expect(mocks.writeConfigFile).toHaveBeenCalledTimes(1); + // Verify readConfigFileSnapshot was called: initial read, after conflict, after successful write + expect(mocks.readConfigFileSnapshot).toHaveBeenCalledTimes(3); + + // Verify plugin-written nested config survived the retry merge. + const retryCall = mocks.replaceConfigFile.mock.calls[1][0] as { + nextConfig: Record; + }; + expect(retryCall.nextConfig).toMatchObject({ + agents: { + defaults: { + workspace: expect.stringContaining("/.openclaw/workspace"), + }, + }, + plugins: { + entries: { + "github-copilot": { + enabled: false, + config: { + region: "us-east-1", + accessToken: "plugin-wrote-this", + }, + }, + }, + }, + }); + }); }); diff --git a/src/commands/configure.wizard.ts b/src/commands/configure.wizard.ts index 21fbb37bad3..e64b7cf2630 100644 --- a/src/commands/configure.wizard.ts +++ b/src/commands/configure.wizard.ts @@ -1,16 +1,18 @@ import fsPromises from "node:fs/promises"; import nodePath from "node:path"; +import { isDeepStrictEqual } from "node:util"; import { describeCodexNativeWebSearch } from "../agents/codex-native-web-search.shared.js"; import { formatCliCommand } from "../cli/command-format.js"; import { readConfigFileSnapshot, replaceConfigFile, resolveGatewayPort } from "../config/config.js"; import { logConfigUpdated } from "../config/logging.js"; +import { ConfigMutationConflictError } from "../config/mutate.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { ensureControlUiAssetsBuilt } from "../infra/control-ui-assets.js"; import type { RuntimeEnv } from "../runtime.js"; import { defaultRuntime } from "../runtime.js"; import { normalizeOptionalString } from "../shared/string-coerce.js"; import { note } from "../terminal/note.js"; -import { resolveUserPath } from "../utils.js"; +import { isPlainObject, resolveUserPath } from "../utils.js"; import { createClackPrompter } from "../wizard/clack-prompter.js"; import { WizardCancelledError } from "../wizard/prompts.js"; import { resolveSetupSecretInputString } from "../wizard/setup.secret-input.js"; @@ -49,6 +51,26 @@ import { setupSkills } from "./onboard-skills.js"; type ConfigureSectionChoice = WizardSection | "__continue"; +function mergeWizardConfigOntoLatest(current: unknown, base: unknown, next: unknown): unknown { + if (isDeepStrictEqual(next, base)) { + return current; + } + if (isPlainObject(current) && isPlainObject(base) && isPlainObject(next)) { + const merged: Record = { ...current }; + const keys = new Set([...Object.keys(current), ...Object.keys(base), ...Object.keys(next)]); + for (const key of keys) { + const mergedValue = mergeWizardConfigOntoLatest(current[key], base[key], next[key]); + if (mergedValue === undefined) { + delete merged[key]; + } else { + merged[key] = mergedValue; + } + } + return merged; + } + return structuredClone(next); +} + async function resolveGatewaySecretInputForWizard(params: { cfg: OpenClawConfig; value: unknown; @@ -426,6 +448,7 @@ export async function runConfigureWizard( } let nextConfig = { ...baseConfig }; + let mergeBaseConfig = structuredClone(baseConfig); let didSetGatewayMode = false; if (nextConfig.gateway?.mode !== "local") { nextConfig = { @@ -448,12 +471,43 @@ export async function runConfigureWizard( command: opts.command, mode, }); - await replaceConfigFile({ - nextConfig, - ...(currentBaseHash !== undefined ? { baseHash: currentBaseHash } : {}), - }); - currentBaseHash = undefined; - logConfigUpdated(runtime); + + // Retry loop: if config was mutated by a plugin, re-read and merge before retry + const maxRetries = 3; + for (let attempt = 0; attempt < maxRetries; attempt++) { + try { + await replaceConfigFile({ + nextConfig, + ...(currentBaseHash !== undefined ? { baseHash: currentBaseHash } : {}), + }); + + // After successful write, re-read the snapshot to get the new hash + const freshSnapshot = await readConfigFileSnapshot(); + currentBaseHash = freshSnapshot.hash ?? undefined; + mergeBaseConfig = structuredClone(nextConfig); + + logConfigUpdated(runtime); + return; + } catch (err) { + if (err instanceof ConfigMutationConflictError && attempt < maxRetries - 1) { + // Config was mutated externally (e.g. plugin wrote token during auth setup). + // Re-read the on-disk config and merge plugin changes into nextConfig so + // the retry won't silently overwrite them. + const freshSnapshot = await readConfigFileSnapshot(); + currentBaseHash = freshSnapshot.hash ?? undefined; + const diskConfig = freshSnapshot.valid + ? (freshSnapshot.sourceConfig ?? freshSnapshot.config) + : {}; + nextConfig = mergeWizardConfigOntoLatest( + diskConfig, + mergeBaseConfig, + nextConfig, + ) as OpenClawConfig; + continue; + } + throw err; + } + } }; const configureWorkspace = async () => {