fix: block invalid config startup
Co-authored-by: Muhammed Mukhthar CM <mukhtharcm@gmail.com>
This commit is contained in:
@@ -57,6 +57,7 @@ Docs: https://docs.clawd.bot
|
||||
- Verbose: wrap tool summaries/output in markdown only for markdown-capable channels.
|
||||
- Telegram: accept tg/group/telegram prefixes + topic targets for inline button validation. (#1072) — thanks @danielz1z.
|
||||
- Telegram: split long captions into follow-up messages.
|
||||
- Config: block startup on invalid config, preserve best-effort doctor config, and keep rolling config backups. (#1083) — thanks @mukhtharcm.
|
||||
- Sub-agents: normalize announce delivery origin + queue bucketing by accountId to keep multi-account routing stable. (#1061, #1058) — thanks @adam91holt.
|
||||
- Sessions: include deliveryContext in sessions.list and reuse normalized delivery routing for announce/restart fallbacks. (#1058)
|
||||
- Sessions: propagate deliveryContext into last-route updates to keep account/channel routing stable. (#1058)
|
||||
|
||||
@@ -61,6 +61,15 @@ export function registerPreActionHooks(program: Command, programVersion: string)
|
||||
|
||||
program.hook("preAction", async (_thisCommand, actionCommand) => {
|
||||
if (actionCommand.name() === "doctor") return;
|
||||
const snapshot = await readConfigFileSnapshot();
|
||||
if (snapshot.exists && !snapshot.valid) {
|
||||
defaultRuntime.error(`Config invalid at ${snapshot.path}.`);
|
||||
for (const issue of snapshot.issues) {
|
||||
defaultRuntime.error(`- ${issue.path || "<root>"}: ${issue.message}`);
|
||||
}
|
||||
defaultRuntime.error("Run `clawdbot doctor` to repair, then retry.");
|
||||
process.exit(1);
|
||||
}
|
||||
const cfg = loadConfig();
|
||||
await autoMigrateLegacyState({ cfg });
|
||||
});
|
||||
|
||||
37
src/commands/doctor-config-flow.test.ts
Normal file
37
src/commands/doctor-config-flow.test.ts
Normal file
@@ -0,0 +1,37 @@
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { withTempHome } from "../../test/helpers/temp-home.js";
|
||||
import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js";
|
||||
|
||||
describe("doctor config flow", () => {
|
||||
it("preserves invalid config for doctor repairs", async () => {
|
||||
await withTempHome(async (home) => {
|
||||
const configDir = path.join(home, ".clawdbot");
|
||||
await fs.mkdir(configDir, { recursive: true });
|
||||
await fs.writeFile(
|
||||
path.join(configDir, "clawdbot.json"),
|
||||
JSON.stringify(
|
||||
{
|
||||
gateway: { auth: { mode: "token", token: 123 } },
|
||||
agents: { list: [{ id: "pi" }] },
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
"utf-8",
|
||||
);
|
||||
|
||||
const result = await loadAndMaybeMigrateDoctorConfig({
|
||||
options: { nonInteractive: true },
|
||||
confirm: async () => false,
|
||||
});
|
||||
|
||||
expect((result.cfg as Record<string, unknown>).gateway).toEqual({
|
||||
auth: { mode: "token", token: 123 },
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -46,9 +46,9 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
|
||||
confirm: (p: { message: string; initialValue: boolean }) => Promise<boolean>;
|
||||
}) {
|
||||
const snapshot = await readConfigFileSnapshot();
|
||||
let cfg: ClawdbotConfig = snapshot.valid ? snapshot.config : {};
|
||||
let cfg: ClawdbotConfig = snapshot.config ?? {};
|
||||
if (snapshot.exists && !snapshot.valid && snapshot.legacyIssues.length === 0) {
|
||||
note("Config invalid; doctor will run with defaults.", "Config");
|
||||
note("Config invalid; doctor will run with best-effort config.", "Config");
|
||||
}
|
||||
|
||||
if (snapshot.legacyIssues.length > 0) {
|
||||
|
||||
36
src/config/config.backup-rotation.test.ts
Normal file
36
src/config/config.backup-rotation.test.ts
Normal file
@@ -0,0 +1,36 @@
|
||||
import fs from "node:fs/promises";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { withTempHome } from "./test-helpers.js";
|
||||
import type { ClawdbotConfig } from "./types.js";
|
||||
|
||||
describe("config backup rotation", () => {
|
||||
it("keeps a 5-deep backup ring for config writes", async () => {
|
||||
await withTempHome(async () => {
|
||||
const { resolveConfigPath, writeConfigFile } = await import("./config.js");
|
||||
const configPath = resolveConfigPath();
|
||||
const buildConfig = (version: number): ClawdbotConfig =>
|
||||
({
|
||||
identity: { name: `v${version}` },
|
||||
}) as ClawdbotConfig;
|
||||
|
||||
for (let version = 0; version <= 6; version += 1) {
|
||||
await writeConfigFile(buildConfig(version));
|
||||
}
|
||||
|
||||
const readName = async (suffix = "") => {
|
||||
const raw = await fs.readFile(`${configPath}${suffix}`, "utf-8");
|
||||
return (JSON.parse(raw) as { identity?: { name?: string } }).identity?.name ?? null;
|
||||
};
|
||||
|
||||
await expect(readName()).resolves.toBe("v6");
|
||||
await expect(readName(".bak")).resolves.toBe("v5");
|
||||
await expect(readName(".bak.1")).resolves.toBe("v4");
|
||||
await expect(readName(".bak.2")).resolves.toBe("v3");
|
||||
await expect(readName(".bak.3")).resolves.toBe("v2");
|
||||
await expect(readName(".bak.4")).resolves.toBe("v1");
|
||||
await expect(fs.stat(`${configPath}.bak.5`)).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -52,6 +52,8 @@ const SHELL_ENV_EXPECTED_KEYS = [
|
||||
"CLAWDBOT_GATEWAY_PASSWORD",
|
||||
];
|
||||
|
||||
const CONFIG_BACKUP_COUNT = 5;
|
||||
|
||||
export type ParseConfigJson5Result = { ok: true; parsed: unknown } | { ok: false; error: string };
|
||||
|
||||
function hashConfigRaw(raw: string | null): string {
|
||||
@@ -73,6 +75,53 @@ export function resolveConfigSnapshotHash(snapshot: {
|
||||
return hashConfigRaw(snapshot.raw);
|
||||
}
|
||||
|
||||
function coerceConfig(value: unknown): ClawdbotConfig {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) {
|
||||
return {};
|
||||
}
|
||||
return value as ClawdbotConfig;
|
||||
}
|
||||
|
||||
function rotateConfigBackupsSync(configPath: string, ioFs: typeof fs): void {
|
||||
if (CONFIG_BACKUP_COUNT <= 1) return;
|
||||
const backupBase = `${configPath}.bak`;
|
||||
const maxIndex = CONFIG_BACKUP_COUNT - 1;
|
||||
try {
|
||||
ioFs.unlinkSync(`${backupBase}.${maxIndex}`);
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
for (let index = maxIndex - 1; index >= 1; index -= 1) {
|
||||
try {
|
||||
ioFs.renameSync(`${backupBase}.${index}`, `${backupBase}.${index + 1}`);
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
}
|
||||
try {
|
||||
ioFs.renameSync(backupBase, `${backupBase}.1`);
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
}
|
||||
|
||||
async function rotateConfigBackups(configPath: string, ioFs: typeof fs.promises): Promise<void> {
|
||||
if (CONFIG_BACKUP_COUNT <= 1) return;
|
||||
const backupBase = `${configPath}.bak`;
|
||||
const maxIndex = CONFIG_BACKUP_COUNT - 1;
|
||||
await ioFs.unlink(`${backupBase}.${maxIndex}`).catch(() => {
|
||||
// best-effort
|
||||
});
|
||||
for (let index = maxIndex - 1; index >= 1; index -= 1) {
|
||||
await ioFs.rename(`${backupBase}.${index}`, `${backupBase}.${index + 1}`).catch(() => {
|
||||
// best-effort
|
||||
});
|
||||
}
|
||||
await ioFs.rename(backupBase, `${backupBase}.1`).catch(() => {
|
||||
// best-effort
|
||||
});
|
||||
}
|
||||
|
||||
export type ConfigIoDeps = {
|
||||
fs?: typeof fs;
|
||||
json5?: typeof JSON5;
|
||||
@@ -165,10 +214,13 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
|
||||
deps.fs.writeFileSync(tmp, json, { encoding: "utf-8", mode: 0o600 });
|
||||
|
||||
try {
|
||||
deps.fs.copyFileSync(configPath, `${configPath}.bak`);
|
||||
} catch {
|
||||
// best-effort
|
||||
if (deps.fs.existsSync(configPath)) {
|
||||
rotateConfigBackupsSync(configPath, deps.fs);
|
||||
try {
|
||||
deps.fs.copyFileSync(configPath, `${configPath}.bak`);
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -344,7 +396,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
raw,
|
||||
parsed: parsedRes.parsed,
|
||||
valid: false,
|
||||
config: {},
|
||||
config: coerceConfig(parsedRes.parsed),
|
||||
hash,
|
||||
issues: [{ path: "", message }],
|
||||
legacyIssues: [],
|
||||
@@ -366,7 +418,7 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
raw,
|
||||
parsed: parsedRes.parsed,
|
||||
valid: false,
|
||||
config: {},
|
||||
config: coerceConfig(resolved),
|
||||
hash,
|
||||
issues: [{ path: "", message }],
|
||||
legacyIssues: [],
|
||||
@@ -379,17 +431,13 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
|
||||
const validated = validateConfigObject(resolvedConfigRaw);
|
||||
if (!validated.ok) {
|
||||
const resolvedConfig =
|
||||
typeof resolvedConfigRaw === "object" && resolvedConfigRaw !== null
|
||||
? (resolvedConfigRaw as ClawdbotConfig)
|
||||
: {};
|
||||
return {
|
||||
path: configPath,
|
||||
exists: true,
|
||||
raw,
|
||||
parsed: parsedRes.parsed,
|
||||
valid: false,
|
||||
config: resolvedConfig,
|
||||
config: coerceConfig(resolvedConfigRaw),
|
||||
hash,
|
||||
issues: validated.issues,
|
||||
legacyIssues,
|
||||
@@ -450,9 +498,12 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
||||
mode: 0o600,
|
||||
});
|
||||
|
||||
await deps.fs.promises.copyFile(configPath, `${configPath}.bak`).catch(() => {
|
||||
// best-effort
|
||||
});
|
||||
if (deps.fs.existsSync(configPath)) {
|
||||
await rotateConfigBackups(configPath, deps.fs.promises);
|
||||
await deps.fs.promises.copyFile(configPath, `${configPath}.bak`).catch(() => {
|
||||
// best-effort
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
await deps.fs.promises.rename(tmp, configPath);
|
||||
|
||||
@@ -134,7 +134,7 @@ export async function startGatewayServer(
|
||||
// Ensure all default port derivations (browser/bridge/canvas) see the actual runtime port.
|
||||
process.env.CLAWDBOT_GATEWAY_PORT = String(port);
|
||||
|
||||
const configSnapshot = await readConfigFileSnapshot();
|
||||
let configSnapshot = await readConfigFileSnapshot();
|
||||
if (configSnapshot.legacyIssues.length > 0) {
|
||||
if (isNixMode) {
|
||||
throw new Error(
|
||||
@@ -157,6 +157,19 @@ export async function startGatewayServer(
|
||||
}
|
||||
}
|
||||
|
||||
configSnapshot = await readConfigFileSnapshot();
|
||||
if (configSnapshot.exists && !configSnapshot.valid) {
|
||||
const issues =
|
||||
configSnapshot.issues.length > 0
|
||||
? configSnapshot.issues
|
||||
.map((issue) => `${issue.path || "<root>"}: ${issue.message}`)
|
||||
.join("\n")
|
||||
: "Unknown validation issue.";
|
||||
throw new Error(
|
||||
`Invalid config at ${configSnapshot.path}.\n${issues}\nRun "clawdbot doctor" to repair, then retry.`,
|
||||
);
|
||||
}
|
||||
|
||||
const cfgAtStart = loadConfig();
|
||||
initSubagentRegistry();
|
||||
await autoMigrateLegacyState({ cfg: cfgAtStart, log });
|
||||
|
||||
Reference in New Issue
Block a user