From ea3a5bf72682ae08ce6e98e2f55056aa610ebefd Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 16 Aug 2026 22:37:08 -0700 Subject: [PATCH] fix(config): record when the post-write canonical reread degrades (#125075) The canonical reread after a committed config write silently kept the in-memory config whenever the reread came back invalid or missing (a concurrent edit racing the commit). Runtime and disk then diverged with no recorded reason. Record the degradation via the config IO logger, including the reread's issue summary. Expose the deps logger on the createConfigIO facade (it already carries logger internally). --- src/config/io.factory.ts | 1 + src/config/io.runtime.ts | 11 +++++ src/config/io.write-reread.test.ts | 70 ++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 src/config/io.write-reread.test.ts diff --git a/src/config/io.factory.ts b/src/config/io.factory.ts index 4b65cdf73a1f..e3bc97609898 100644 --- a/src/config/io.factory.ts +++ b/src/config/io.factory.ts @@ -24,6 +24,7 @@ export function createConfigIO(options: ConfigIoFactoryOptions = {}) { return { configPath: context.configPath, env: context.deps.env, + logger: context.deps.logger, loadConfig: (loadOptions?: { skipSuspiciousRecovery?: boolean }) => loadConfigFromContext(context, loadOptions), readBestEffortConfig: async () => diff --git a/src/config/io.runtime.ts b/src/config/io.runtime.ts index 6ee53c98b801..729d26c8b28e 100644 --- a/src/config/io.runtime.ts +++ b/src/config/io.runtime.ts @@ -24,6 +24,7 @@ import type { } from "./io.types.js"; import { ConfigRuntimeRefreshError, configWritePostCommitRollback } from "./io.types.js"; import { rollbackConfigFileWriteIfUnchanged } from "./io.write-safety.js"; +import { formatConfigIssueSummary } from "./issue-format.js"; import { applyMergePatch, createMergePatch } from "./merge-patch.js"; import { ConfigMutationConflictError } from "./mutation-conflict.js"; import { assertConfigWriteAllowedInCurrentMode } from "./nix-mode-write-guard.js"; @@ -390,6 +391,16 @@ async function finalizeCommittedConfigWrite(params: { if (freshSnapshot.exists && freshSnapshot.valid) { canonicalSourceConfig = freshSnapshot.sourceConfig; canonicalRuntimeConfig = freshSnapshot.config; + } else { + // An invalid or vanished reread means a concurrent edit beat us to the + // file; runtime keeps the just-written config, but that divergence must + // be recorded or the on-disk config silently stops matching runtime. + const issueSummary = formatConfigIssueSummary(freshSnapshot.issues); + io.logger.warn( + `Config (${io.configPath}): canonical reread after write was ${ + freshSnapshot.exists ? "invalid" : "missing" + }; runtime keeps the written config${issueSummary ? `: ${issueSummary}` : ""}`, + ); } if ( !deferRuntimeActivation || diff --git a/src/config/io.write-reread.test.ts b/src/config/io.write-reread.test.ts new file mode 100644 index 000000000000..ccac478139af --- /dev/null +++ b/src/config/io.write-reread.test.ts @@ -0,0 +1,70 @@ +// Covers the canonical reread that follows a committed config write. +import fsNode from "node:fs"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; +import { withEnvAsync } from "../test-utils/env.js"; +import { writeConfigFile } from "./io.runtime.js"; +import { + clearRuntimeConfigSnapshot, + setRuntimeConfigSnapshotRefreshHandler, +} from "./runtime-snapshot.js"; +import { withTempHome } from "./test-helpers.js"; + +describe("writeConfigFile canonical reread", () => { + afterEach(() => { + setRuntimeConfigSnapshotRefreshHandler(null); + clearRuntimeConfigSnapshot(); + closeOpenClawStateDatabaseForTest(); + vi.restoreAllMocks(); + }); + + it("records when the post-write reread is invalid instead of silently keeping runtime state", async () => { + await withTempHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile( + configPath, + `${JSON.stringify({ gateway: { mode: "local", port: 18789 } }, null, 2)}\n`, + "utf-8", + ); + + // Simulate a concurrent edit racing the commit: after the write renames the + // new config into place, every subsequent sync read sees corrupt content, + // so the canonical reread parses invalid. + let corrupted = false; + const realRename = fsNode.promises.rename.bind(fsNode.promises); + vi.spyOn(fsNode.promises, "rename").mockImplementation(async (from, to) => { + await realRename(from, to); + if (to === configPath) { + corrupted = true; + } + }); + const realReadFileSync = fsNode.readFileSync.bind(fsNode); + vi.spyOn(fsNode, "readFileSync").mockImplementation(((target, options) => { + if (corrupted && target === configPath) { + return "{ definitely not json"; + } + return realReadFileSync(target as Parameters[0], options); + }) as typeof fsNode.readFileSync); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + // Keep runtime-snapshot finalization from re-parsing the corrupt file; + // this test targets only the canonical reread's recorded degradation. + setRuntimeConfigSnapshotRefreshHandler({ refresh: async () => true }); + + await withEnvAsync( + { OPENCLAW_CONFIG_PATH: configPath, OPENCLAW_TEST_FAST: "1" }, + async () => { + await writeConfigFile({ gateway: { mode: "local", port: 19001 } }); + }, + ); + + expect( + warn.mock.calls.some(([line]) => + String(line).includes("canonical reread after write was invalid"), + ), + ).toBe(true); + }); + }); +});