fix(doctor): surface legacy-config copy failures (#124948)

* 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.
This commit is contained in:
Peter Steinberger
2026-08-16 19:04:19 -07:00
committed by GitHub
parent 89f6ab109b
commit a23d19218c
2 changed files with 94 additions and 2 deletions
@@ -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<string, string | undefined>();
function setEnv(values: Partial<Record<(typeof envKeys)[number], string>>) {
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");
});
});
});
+12 -2
View File
@@ -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<string[]> {
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;