mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 11:25:50 -06:00
fix(cli): offer config repair after invalid startup (#110533)
* fix(cli): offer doctor recovery for invalid config * fix(cli): preserve invalid config overrides * fix(cli): keep invalid config recovery controlled
This commit is contained in:
committed by
GitHub
parent
ccf55a43cd
commit
1710313596
@@ -96,12 +96,14 @@ describe("ensureConfigReady", () => {
|
||||
snapshot,
|
||||
baseConfig: {},
|
||||
});
|
||||
return snapshot;
|
||||
}
|
||||
|
||||
function useTempOpenClawHome(): string {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-config-guard-"));
|
||||
tempRoots.push(root);
|
||||
setTestEnvValue("OPENCLAW_HOME", root);
|
||||
deleteTestEnvValue("OPENCLAW_NIX_MODE");
|
||||
deleteTestEnvValue("OPENCLAW_PROFILE");
|
||||
deleteTestEnvValue("OPENCLAW_STATE_DIR");
|
||||
return root;
|
||||
@@ -127,7 +129,13 @@ describe("ensureConfigReady", () => {
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
envSnapshot = captureEnv(["HOME", "OPENCLAW_HOME", "OPENCLAW_PROFILE", "OPENCLAW_STATE_DIR"]);
|
||||
envSnapshot = captureEnv([
|
||||
"HOME",
|
||||
"OPENCLAW_HOME",
|
||||
"OPENCLAW_NIX_MODE",
|
||||
"OPENCLAW_PROFILE",
|
||||
"OPENCLAW_STATE_DIR",
|
||||
]);
|
||||
vi.clearAllMocks();
|
||||
resetConfigGuardStateForTests();
|
||||
for (const root of tempRoots.splice(0)) {
|
||||
@@ -505,13 +513,87 @@ describe("ensureConfigReady", () => {
|
||||
"Problem:",
|
||||
" - channels.quietchat: invalid",
|
||||
"",
|
||||
`Fix: ${formatCliCommand("openclaw doctor --fix")}`,
|
||||
`Inspect: ${formatCliCommand("openclaw config validate")}`,
|
||||
"Audit, status, health, logs, tasks list/audit, and doctor commands still run with invalid config.",
|
||||
`Run "${formatCliCommand("openclaw doctor --fix")}" to repair the config, then retry.`,
|
||||
]);
|
||||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it("runs doctor and retries the config guard once after consent", async () => {
|
||||
writeLegacyTaskSidecarMarker(useTempOpenClawHome());
|
||||
const invalidSnapshot = setInvalidSnapshot();
|
||||
const validSnapshot = {
|
||||
...makeSnapshot(),
|
||||
config: { gateway: { mode: "local" } },
|
||||
sourceConfig: { gateway: { mode: "local" } },
|
||||
};
|
||||
loadAndMaybeMigrateDoctorConfigMock
|
||||
.mockResolvedValueOnce({ snapshot: invalidSnapshot, baseConfig: {} })
|
||||
.mockResolvedValueOnce({ snapshot: validSnapshot, baseConfig: validSnapshot.config });
|
||||
readConfigFileSnapshotMock.mockResolvedValue(validSnapshot);
|
||||
const runtime = makeRuntime();
|
||||
const confirm = vi.fn(async () => true);
|
||||
const runDoctor = vi.fn(async () => {});
|
||||
|
||||
await ensureConfigReady(
|
||||
{ runtime: runtime as never, commandPath: ["message"] },
|
||||
{ confirm, isInteractive: () => true, runDoctor },
|
||||
);
|
||||
|
||||
expect(confirm).toHaveBeenCalledWith(
|
||||
`Run "${formatCliCommand("openclaw doctor --fix")}" now?`,
|
||||
true,
|
||||
);
|
||||
expect(runDoctor).toHaveBeenCalledOnce();
|
||||
expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenCalledTimes(2);
|
||||
expect(loadAndMaybeMigrateDoctorConfigMock).toHaveBeenLastCalledWith({
|
||||
migrateState: false,
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
});
|
||||
expect(readConfigFileSnapshotMock).not.toHaveBeenCalled();
|
||||
expect(setRuntimeConfigSnapshotMock).toHaveBeenCalledWith(
|
||||
validSnapshot.config,
|
||||
validSnapshot.sourceConfig,
|
||||
);
|
||||
expect(runtime.exit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not prompt for repair when stdout belongs to a machine-readable command", async () => {
|
||||
setInvalidSnapshot();
|
||||
const runtime = makeRuntime();
|
||||
const confirm = vi.fn(async () => true);
|
||||
|
||||
await ensureConfigReady(
|
||||
{
|
||||
runtime: runtime as never,
|
||||
commandPath: ["agents", "list"],
|
||||
suppressDoctorStdout: true,
|
||||
},
|
||||
{ confirm, isInteractive: () => true },
|
||||
);
|
||||
|
||||
expect(confirm).not.toHaveBeenCalled();
|
||||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it("keeps invalid Nix-managed config on the manual recovery path", async () => {
|
||||
setInvalidSnapshot();
|
||||
setTestEnvValue("OPENCLAW_NIX_MODE", "1");
|
||||
const runtime = makeRuntime();
|
||||
const confirm = vi.fn(async () => true);
|
||||
|
||||
await ensureConfigReady(
|
||||
{ runtime: runtime as never, commandPath: ["gateway", "run"] },
|
||||
{ confirm, isInteractive: () => true },
|
||||
);
|
||||
|
||||
expect(confirm).not.toHaveBeenCalled();
|
||||
expect(plainErrorCalls(runtime).join("\n")).toContain("Config is managed by Nix");
|
||||
expect(runtime.exit).toHaveBeenCalledWith(78);
|
||||
});
|
||||
|
||||
it("replaces doctor fix advice for plugin packaging-only invalid config", async () => {
|
||||
setInvalidSnapshot({
|
||||
issues: [
|
||||
@@ -534,9 +616,12 @@ describe("ensureConfigReady", () => {
|
||||
expect(calls).toContain(`Fix: ${pluginPackagingRecoveryHint}`);
|
||||
expect(calls).not.toContain(`Fix: ${formatCliCommand("openclaw doctor --fix")}`);
|
||||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
|
||||
const gatewayRuntime = await runEnsureConfigReady(["gateway", "start"]);
|
||||
expect(gatewayRuntime.exit).toHaveBeenCalledWith(78);
|
||||
});
|
||||
|
||||
it("does not exit for invalid config on allowlisted commands", async () => {
|
||||
it("allows read-only invalid-config commands but blocks gateway startup", async () => {
|
||||
setInvalidSnapshot({
|
||||
issues: [{ path: "agents.defaults", message: 'Unrecognized key: "agentRuntime"' }],
|
||||
});
|
||||
@@ -547,10 +632,16 @@ describe("ensureConfigReady", () => {
|
||||
expect(auditRuntime.exit).not.toHaveBeenCalled();
|
||||
|
||||
const bareGatewayRuntime = await runEnsureConfigReady(["gateway"]);
|
||||
expect(bareGatewayRuntime.exit).not.toHaveBeenCalled();
|
||||
expect(bareGatewayRuntime.exit).toHaveBeenCalledWith(78);
|
||||
|
||||
const gatewayRunRuntime = await runEnsureConfigReady(["gateway", "run"]);
|
||||
expect(gatewayRunRuntime.exit).not.toHaveBeenCalled();
|
||||
expect(gatewayRunRuntime.exit).toHaveBeenCalledWith(78);
|
||||
|
||||
const gatewayStartRuntime = await runEnsureConfigReady(["gateway", "start"]);
|
||||
expect(gatewayStartRuntime.exit).toHaveBeenCalledWith(78);
|
||||
|
||||
const gatewayRestartRuntime = await runEnsureConfigReady(["gateway", "restart"]);
|
||||
expect(gatewayRestartRuntime.exit).toHaveBeenCalledWith(78);
|
||||
|
||||
const gatewayRuntime = await runEnsureConfigReady(["gateway", "health"]);
|
||||
expect(gatewayRuntime.exit).not.toHaveBeenCalled();
|
||||
@@ -583,6 +674,24 @@ describe("ensureConfigReady", () => {
|
||||
expect(runtime.exit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not offer repair for an explicitly allowed gateway startup", async () => {
|
||||
setInvalidSnapshot();
|
||||
const runtime = makeRuntime();
|
||||
const confirm = vi.fn(async () => true);
|
||||
|
||||
await ensureConfigReady(
|
||||
{
|
||||
runtime: runtime as never,
|
||||
commandPath: ["gateway", "run"],
|
||||
allowInvalid: true,
|
||||
},
|
||||
{ confirm, isInteractive: () => true },
|
||||
);
|
||||
|
||||
expect(confirm).not.toHaveBeenCalled();
|
||||
expect(runtime.exit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("runs doctor migration flow only once per module instance", async () => {
|
||||
writeLegacyTaskSidecarMarker(useTempOpenClawHome());
|
||||
const runtimeA = makeRuntime();
|
||||
|
||||
@@ -4,11 +4,18 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { withSuppressedNotes } from "../../../packages/terminal-core/src/note.js";
|
||||
import { readConfigFileSnapshot, setRuntimeConfigSnapshot } from "../../config/config.js";
|
||||
import { resolveLegacyStateDirs, resolveOAuthDir, resolveStateDir } from "../../config/paths.js";
|
||||
import { createInvalidConfigError } from "../../config/io.invalid-config.js";
|
||||
import {
|
||||
resolveIsNixMode,
|
||||
resolveLegacyStateDirs,
|
||||
resolveOAuthDir,
|
||||
resolveStateDir,
|
||||
} from "../../config/paths.js";
|
||||
import type { ConfigFileSnapshot } from "../../config/types.js";
|
||||
import { resolveRequiredHomeDir } from "../../infra/home-dir.js";
|
||||
import { ExitError, type RuntimeEnv } from "../../runtime.js";
|
||||
import { shouldMigrateStateFromPath } from "../argv.js";
|
||||
import type { InvalidConfigRecoveryDeps } from "../invalid-config-recovery.js";
|
||||
|
||||
const ALLOWED_INVALID_COMMANDS = new Set(["audit", "doctor", "logs", "health", "help", "status"]);
|
||||
const ALLOWED_INVALID_GATEWAY_SUBCOMMANDS = new Set([
|
||||
@@ -167,6 +174,17 @@ function shouldRequireStartupMigrationCheckpoint(commandPath: string[]): boolean
|
||||
);
|
||||
}
|
||||
|
||||
function isGatewayStartupCommand(commandPath: string[]): boolean {
|
||||
const [commandName, subcommandName] = commandPath;
|
||||
return (
|
||||
commandName === "gateway" &&
|
||||
(subcommandName === undefined ||
|
||||
subcommandName === "run" ||
|
||||
subcommandName === "start" ||
|
||||
subcommandName === "restart")
|
||||
);
|
||||
}
|
||||
|
||||
async function getConfigSnapshot(options?: { observe: false }) {
|
||||
if (options?.observe === false) {
|
||||
return readConfigFileSnapshot(options);
|
||||
@@ -187,15 +205,18 @@ async function getConfigSnapshot(options?: { observe: false }) {
|
||||
return configSnapshotPromise;
|
||||
}
|
||||
|
||||
export async function ensureConfigReady(params: {
|
||||
runtime: RuntimeEnv;
|
||||
commandPath?: string[];
|
||||
suppressDoctorStdout?: boolean;
|
||||
allowInvalid?: boolean;
|
||||
beforeStateMigrations?: (snapshot?: ConfigFileSnapshot) => Promise<boolean>;
|
||||
skipPristineCoreStateMigrations?: boolean;
|
||||
skipPristineStartupStateMigrations?: boolean;
|
||||
}): Promise<void> {
|
||||
export async function ensureConfigReady(
|
||||
params: {
|
||||
runtime: RuntimeEnv;
|
||||
commandPath?: string[];
|
||||
suppressDoctorStdout?: boolean;
|
||||
allowInvalid?: boolean;
|
||||
beforeStateMigrations?: (snapshot?: ConfigFileSnapshot) => Promise<boolean>;
|
||||
skipPristineCoreStateMigrations?: boolean;
|
||||
skipPristineStartupStateMigrations?: boolean;
|
||||
},
|
||||
recoveryDeps?: InvalidConfigRecoveryDeps,
|
||||
): Promise<void> {
|
||||
const commandPath = params.commandPath ?? [];
|
||||
const commandName = commandPath[0];
|
||||
const subcommandName = commandPath[1];
|
||||
@@ -319,10 +340,22 @@ export async function ensureConfigReady(params: {
|
||||
params.runtime.error(legacyIssues.map((issue) => ` ${error(issue)}`).join("\n"));
|
||||
}
|
||||
params.runtime.error("");
|
||||
const fixHint = isPluginPackagingRuntimeOutputInvalidConfigSnapshot(snapshot)
|
||||
? formatPluginPackagingRuntimeOutputRecoveryHint()
|
||||
: commandText(formatCliCommand("openclaw doctor --fix"));
|
||||
params.runtime.error(`${muted("Fix:")} ${fixHint}`);
|
||||
const isPluginPackagingFailure = isPluginPackagingRuntimeOutputInvalidConfigSnapshot(snapshot);
|
||||
const isNixManagedConfig = resolveIsNixMode();
|
||||
const isGatewayStartup = isGatewayStartupCommand(commandPath);
|
||||
const mustBlockInvalid = !allowInvalid || (isGatewayStartup && params.allowInvalid !== true);
|
||||
const shouldOfferRecovery =
|
||||
mustBlockInvalid && !params.suppressDoctorStdout && !isNixManagedConfig;
|
||||
if (isPluginPackagingFailure || isNixManagedConfig || !shouldOfferRecovery) {
|
||||
const fixHint = isPluginPackagingFailure
|
||||
? formatPluginPackagingRuntimeOutputRecoveryHint()
|
||||
: isNixManagedConfig
|
||||
? new (await import("../../config/nix-mode-write-guard.js")).NixModeConfigMutationError({
|
||||
configPath: snapshot.path,
|
||||
}).message
|
||||
: commandText(formatCliCommand("openclaw doctor --fix"));
|
||||
params.runtime.error(`${muted("Fix:")} ${fixHint}`);
|
||||
}
|
||||
params.runtime.error(
|
||||
`${muted("Inspect:")} ${commandText(formatCliCommand("openclaw config validate"))}`,
|
||||
);
|
||||
@@ -331,8 +364,52 @@ export async function ensureConfigReady(params: {
|
||||
"Audit, status, health, logs, tasks list/audit, and doctor commands still run with invalid config.",
|
||||
),
|
||||
);
|
||||
if (!allowInvalid) {
|
||||
params.runtime.exit(1);
|
||||
if (isPluginPackagingFailure && isGatewayStartup) {
|
||||
params.runtime.exit(78);
|
||||
return;
|
||||
}
|
||||
if (shouldOfferRecovery && !isPluginPackagingFailure) {
|
||||
const { offerInvalidConfigRecovery } = await import("../invalid-config-recovery.js");
|
||||
const recovery = await offerInvalidConfigRecovery({
|
||||
runtime: params.runtime,
|
||||
deps: recoveryDeps,
|
||||
retry: async () => {
|
||||
// Doctor may rewrite config; retry the same legacy/plugin-aware validation without
|
||||
// rerunning startup state migrations.
|
||||
configSnapshotPromise = null;
|
||||
const { runDoctorConfigPreflight } =
|
||||
await import("../../commands/doctor-config-preflight.js");
|
||||
const retrySnapshot = (
|
||||
await runDoctorConfigPreflight({
|
||||
migrateState: false,
|
||||
migrateLegacyConfig: false,
|
||||
invalidConfigNote: false,
|
||||
...configSnapshotOptions,
|
||||
})
|
||||
).snapshot;
|
||||
if (retrySnapshot.exists && !retrySnapshot.valid) {
|
||||
const retryIssues = formatConfigIssueLines(retrySnapshot.issues, "-", {
|
||||
normalizeRoot: true,
|
||||
});
|
||||
throw createInvalidConfigError(
|
||||
retrySnapshot.path,
|
||||
retryIssues.join("\n") || "Unknown validation issue.",
|
||||
);
|
||||
}
|
||||
setRuntimeConfigSnapshot(
|
||||
retrySnapshot.runtimeConfig ?? retrySnapshot.config,
|
||||
retrySnapshot.sourceConfig,
|
||||
);
|
||||
},
|
||||
});
|
||||
if (recovery.status === "recovered") {
|
||||
return;
|
||||
}
|
||||
params.runtime.exit(isGatewayStartup ? 78 : 1);
|
||||
return;
|
||||
}
|
||||
if (mustBlockInvalid) {
|
||||
params.runtime.exit(isGatewayStartup ? 78 : 1);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ const DISCORD_REPO_INSTALL_SPEC = repoInstallSpec("discord");
|
||||
const setVerboseMock = vi.fn();
|
||||
const emitCliBannerMock = vi.fn();
|
||||
type EnsureConfigReadyOptions = {
|
||||
allowInvalid?: boolean;
|
||||
beforeStateMigrations?: () => Promise<boolean>;
|
||||
commandPath?: string[];
|
||||
requireConfig?: boolean;
|
||||
@@ -167,11 +168,13 @@ describe("registerPreActionHooks", () => {
|
||||
.action(() => {});
|
||||
const gateway = programLocal
|
||||
.command("gateway")
|
||||
.option("--allow-unconfigured")
|
||||
.option("--force")
|
||||
.option("--reset")
|
||||
.action(() => {});
|
||||
gateway
|
||||
.command("run")
|
||||
.option("--allow-unconfigured")
|
||||
.option("--force")
|
||||
.option("--reset")
|
||||
.action(() => {});
|
||||
@@ -340,6 +343,26 @@ describe("registerPreActionHooks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("passes --allow-unconfigured through as an invalid-config override", async () => {
|
||||
const gatewayRunCommand = resolveActionCommand(["gateway", "run"]);
|
||||
gatewayRunCommand.setOptionValueWithSource("allowUnconfigured", true, "cli");
|
||||
try {
|
||||
await runPreAction({
|
||||
parseArgv: ["gateway", "run"],
|
||||
processArgv: ["node", "openclaw", "gateway", "run", "--allow-unconfigured"],
|
||||
});
|
||||
} finally {
|
||||
gatewayRunCommand.setOptionValueWithSource("allowUnconfigured", false, "default");
|
||||
}
|
||||
|
||||
expect(ensureConfigReadyMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
allowInvalid: true,
|
||||
commandPath: ["gateway", "run"],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("loads plugins for text local agent runs", async () => {
|
||||
await runPreAction({
|
||||
parseArgv: ["agent"],
|
||||
|
||||
@@ -144,6 +144,7 @@ export function registerPreActionHooks(program: Command, programVersion: string)
|
||||
let beforeStateMigrations: ((snapshot?: ConfigFileSnapshot) => Promise<boolean>) | undefined;
|
||||
let skipPristineStartupStateMigrations = false;
|
||||
let skipPristineCoreStateMigrations = false;
|
||||
let allowInvalid = shouldAllowInvalidConfigForAction(actionCommand, commandPath);
|
||||
if (isGatewayRunAction(actionCommand)) {
|
||||
const {
|
||||
prepareGatewayRunBootstrap,
|
||||
@@ -153,6 +154,7 @@ export function registerPreActionHooks(program: Command, programVersion: string)
|
||||
} = await import("../gateway-cli/pre-bootstrap.js");
|
||||
const { resolveGatewayRunOptions } = await import("../gateway-cli/run-options.js");
|
||||
const resolvedOptions = resolveGatewayRunOptions(actionCommand.opts(), actionCommand);
|
||||
allowInvalid ||= resolvedOptions.allowUnconfigured === true;
|
||||
const opts = {
|
||||
force: resolvedOptions.force === true,
|
||||
reset: resolvedOptions.reset === true,
|
||||
@@ -174,7 +176,7 @@ export function registerPreActionHooks(program: Command, programVersion: string)
|
||||
runtime: defaultRuntime,
|
||||
commandPath,
|
||||
startupPolicy,
|
||||
allowInvalid: shouldAllowInvalidConfigForAction(actionCommand, commandPath),
|
||||
allowInvalid,
|
||||
...(beforeStateMigrations ? { beforeStateMigrations } : {}),
|
||||
...(skipPristineStartupStateMigrations ? { skipPristineStartupStateMigrations: true } : {}),
|
||||
...(skipPristineCoreStateMigrations ? { skipPristineCoreStateMigrations: true } : {}),
|
||||
|
||||
Reference in New Issue
Block a user