fix(configure): re-read config hash after persist to avoid stale-hash race (#64188)

This commit is contained in:
Pengfei Ni
2026-04-14 12:12:36 +00:00
committed by Vincent Koc
parent e99a24d645
commit 400d4d26c2
2 changed files with 104 additions and 6 deletions

View File

@@ -109,6 +109,15 @@ vi.mock("./onboard-search.js", () => ({
setupSearch: mocks.setupSearch,
}));
vi.mock("../config/mutate.js", async () => {
const actual = await vi.importActual<typeof import("../config/mutate.js")>("../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,66 @@ describe("runConfigureWizard", () => {
);
expect(mocks.setupSearch).toHaveBeenCalledOnce();
});
it("retries when plugin mutates config during wizard flow (issue #64188)", async () => {
setupBaseWizardState();
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);
});
// Mock readConfigFileSnapshot to return different hashes/configs on each call
mocks.readConfigFileSnapshot
.mockResolvedValueOnce({
...EMPTY_CONFIG_SNAPSHOT,
hash: originalHash,
config: {},
sourceConfig: {},
})
.mockResolvedValueOnce({
...EMPTY_CONFIG_SNAPSHOT,
hash: newHashAfterMutation,
config: { ai: { copilotToken: "plugin-wrote-this" } },
sourceConfig: { ai: { copilotToken: "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 changes were merged into the retry call's nextConfig
const retryCall = mocks.replaceConfigFile.mock.calls[1][0] as { nextConfig: Record<string, unknown> };
expect((retryCall.nextConfig as Record<string, unknown>).ai).toBeDefined();
});
});

View File

@@ -4,6 +4,7 @@ import { describeCodexNativeWebSearch } from "../agents/codex-native-web-search.
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";
@@ -448,12 +449,38 @@ 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;
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 = { ...diskConfig, ...nextConfig };
continue;
}
throw err;
}
}
};
const configureWorkspace = async () => {