From a23d19218c5b6605315a8e90bb1fe2bbf989dfa9 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 16 Aug 2026 19:04:19 -0700 Subject: [PATCH] fix(doctor): surface legacy-config copy failures (#124948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(doctor): surface legacy-config copy failures maybeMigrateLegacyConfig swallowed every copyFile error with a bare catch whose comment claimed EEXIST-only. EACCES, ENOSPC, and cross-device failures were silently ignored: the operator's ~/.clawdbot/clawdbot.json exists, migration was attempted and failed, and doctor proceeded as a clean fresh install with no config, no change note, and no warning. Root cause: failure collapsed into the skip-silently success shape. The catch now rethrows anything other than EEXIST with the source/target paths in the message; EEXIST (config already at the target) keeps its skip semantics. Regression: read-only target dir makes the preflight reject with 'Failed to migrate legacy config' — fails pre-fix (resolved as a clean run); companion test proves the successful copy path still migrates. * chore(doctor): annotate the errno assertion for the safety ratchet * refactor(doctor): narrow the copy error without a type assertion The assertion-safety ratchet flagged the errno cast; property narrowing removes the assertion instead of annotating it. --- ...octor-config-preflight.legacy-copy.test.ts | 82 +++++++++++++++++++ src/commands/doctor-config-preflight.ts | 14 +++- 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 src/commands/doctor-config-preflight.legacy-copy.test.ts diff --git a/src/commands/doctor-config-preflight.legacy-copy.test.ts b/src/commands/doctor-config-preflight.legacy-copy.test.ts new file mode 100644 index 000000000000..dcfe3a361358 --- /dev/null +++ b/src/commands/doctor-config-preflight.legacy-copy.test.ts @@ -0,0 +1,82 @@ +// A failing legacy-config copy must surface, not silently leave doctor +// looking like a clean fresh install while the operator's config exists. +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { withTempDir } from "../test-utils/temp-dir.js"; +import { runDoctorConfigPreflight } from "./doctor-config-preflight.js"; + +const envKeys = ["HOME", "OPENCLAW_CONFIG_PATH", "OPENCLAW_STATE_DIR"] as const; +const savedEnv = new Map(); + +function setEnv(values: Partial>) { + for (const key of envKeys) { + if (!savedEnv.has(key)) { + savedEnv.set(key, process.env[key]); + } + const value = values[key]; + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } +} + +afterEach(() => { + for (const [key, value] of savedEnv) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + savedEnv.clear(); +}); + +describe("doctor legacy config migration failures", () => { + it.runIf(process.platform !== "win32" && process.getuid?.() !== 0)( + "surfaces a copy failure instead of proceeding as a fresh install", + async () => { + await withTempDir("openclaw-doctor-legacy-copy-", async (home) => { + const legacyDir = path.join(home, ".clawdbot"); + await fs.mkdir(legacyDir, { recursive: true }); + await fs.writeFile(path.join(legacyDir, "clawdbot.json"), "{}\n", "utf-8"); + const targetDir = path.join(home, "readonly-state"); + await fs.mkdir(targetDir, { recursive: true }); + await fs.chmod(targetDir, 0o555); + setEnv({ + HOME: home, + OPENCLAW_CONFIG_PATH: path.join(targetDir, "openclaw.json"), + OPENCLAW_STATE_DIR: path.join(home, "state"), + }); + + try { + await expect( + runDoctorConfigPreflight({ migrateState: false, invalidConfigNote: false }), + ).rejects.toThrow(/Failed to migrate legacy config/); + } finally { + await fs.chmod(targetDir, 0o755); + } + }); + }, + ); + + it("migrates the legacy config and reports the change when the copy works", async () => { + await withTempDir("openclaw-doctor-legacy-copy-", async (home) => { + const legacyDir = path.join(home, ".clawdbot"); + await fs.mkdir(legacyDir, { recursive: true }); + await fs.writeFile(path.join(legacyDir, "clawdbot.json"), "{}\n", "utf-8"); + const targetPath = path.join(home, "state-root", "openclaw.json"); + setEnv({ + HOME: home, + OPENCLAW_CONFIG_PATH: targetPath, + OPENCLAW_STATE_DIR: path.join(home, "state"), + }); + + await runDoctorConfigPreflight({ migrateState: false, invalidConfigNote: false }); + + await expect(fs.readFile(targetPath, "utf-8")).resolves.toBe("{}\n"); + }); + }); +}); diff --git a/src/commands/doctor-config-preflight.ts b/src/commands/doctor-config-preflight.ts index 841a68804429..a4e9b0423311 100644 --- a/src/commands/doctor-config-preflight.ts +++ b/src/commands/doctor-config-preflight.ts @@ -16,6 +16,7 @@ import { resolveCanonicalConfigPath } from "../config/paths.js"; import type { ConfigFileSnapshot } from "../config/types.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { isTruthyEnvValue } from "../infra/env.js"; +import { formatErrorMessage } from "../infra/errors.js"; import type { MigrationCheckpointIdentity, StartupMigrationLease, @@ -99,8 +100,17 @@ async function maybeMigrateLegacyConfig(): Promise { try { await fs.copyFile(legacyPath, targetPath, fs.constants.COPYFILE_EXCL); changes.push(`Migrated legacy config: ${legacyPath} -> ${targetPath}`); - } catch { - // If it already exists, skip silently. + } catch (err) { + // EEXIST means a config already lives at the target — nothing to migrate. + // Any other failure (EACCES, ENOSPC) must surface: doctor would otherwise + // proceed as a fresh install while the operator's legacy config exists. + const code = err && typeof err === "object" && "code" in err ? err.code : undefined; + if (code !== "EEXIST") { + throw new Error( + `Failed to migrate legacy config ${legacyPath} -> ${targetPath}: ${formatErrorMessage(err)}`, + { cause: err }, + ); + } } return changes;