mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-22 18:35:21 -06:00
fix(configure): persist changes before side effects (#125751)
Local mode is persisted before health and daemon actions, and failed actions no longer receive a bare success outro.
This commit is contained in:
committed by
GitHub
parent
9204ab8dc4
commit
227230024d
@@ -84,8 +84,8 @@ function expectUnknownChannelRemovalPrompt(unsafeChannel: string, label: string)
|
||||
`Delete ${label} configuration from ~/.openclaw/openclaw.json?`,
|
||||
);
|
||||
expect(note).toHaveBeenCalledWith(
|
||||
`${label} removed from config.\nNote: credentials/sessions on disk are unchanged.`,
|
||||
"Channel removed",
|
||||
`${label} selected for removal from config.\nNote: credentials/sessions on disk are unchanged.`,
|
||||
"Channel removal",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -143,8 +143,8 @@ describe("removeChannelConfigWizard", () => {
|
||||
);
|
||||
expect(next.channels).toEqual({ twitch: { token: "secret" } });
|
||||
expect(note).toHaveBeenCalledWith(
|
||||
"Telegram removed from config.\nNote: credentials/sessions on disk are unchanged.",
|
||||
"Channel removed",
|
||||
"Telegram selected for removal from config.\nNote: credentials/sessions on disk are unchanged.",
|
||||
"Channel removal",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -164,8 +164,8 @@ describe("removeChannelConfigWizard", () => {
|
||||
expect(confirmArg().message).toBe("Delete done configuration from ~/.openclaw/openclaw.json?");
|
||||
expect(next.channels).toEqual({ telegram: { token: "secret" } });
|
||||
expect(note).toHaveBeenCalledWith(
|
||||
"done removed from config.\nNote: credentials/sessions on disk are unchanged.",
|
||||
"Channel removed",
|
||||
"done selected for removal from config.\nNote: credentials/sessions on disk are unchanged.",
|
||||
"Channel removal",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -231,8 +231,8 @@ describe("removeChannelConfigWizard", () => {
|
||||
"Delete Telegram\\nBot configuration from ~/.openclaw/openclaw.json?",
|
||||
);
|
||||
expect(note).toHaveBeenCalledWith(
|
||||
"Telegram\\nBot removed from config.\nNote: credentials/sessions on disk are unchanged.",
|
||||
"Channel removed",
|
||||
"Telegram\\nBot selected for removal from config.\nNote: credentials/sessions on disk are unchanged.",
|
||||
"Channel removal",
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -130,10 +130,11 @@ export async function removeChannelConfigWizard(
|
||||
}
|
||||
|
||||
note(
|
||||
[`${label} removed from config.`, "Note: credentials/sessions on disk are unchanged."].join(
|
||||
"\n",
|
||||
),
|
||||
"Channel removed",
|
||||
[
|
||||
`${label} selected for removal from config.`,
|
||||
"Note: credentials/sessions on disk are unchanged.",
|
||||
].join("\n"),
|
||||
"Channel removal",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -121,11 +121,12 @@ describe("maybeInstallDaemon", () => {
|
||||
warnings: [],
|
||||
});
|
||||
|
||||
await maybeInstallDaemon({
|
||||
const outcome = await maybeInstallDaemon({
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
port: 18789,
|
||||
});
|
||||
|
||||
expect(outcome).toBe("failed");
|
||||
expect(note).toHaveBeenCalledWith(
|
||||
"Gateway service install failed: Gateway install blocked: gateway.auth.token SecretRef is configured but unresolved (boom). Fix gateway auth config/token input and rerun configure.",
|
||||
"Gateway",
|
||||
@@ -139,12 +140,10 @@ describe("maybeInstallDaemon", () => {
|
||||
new Error("systemctl is-enabled unavailable: Failed to connect to bus"),
|
||||
);
|
||||
|
||||
await expect(
|
||||
maybeInstallDaemon({
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
port: 18789,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
await maybeInstallDaemon({
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
port: 18789,
|
||||
});
|
||||
|
||||
expect(serviceInstall).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
@@ -169,12 +168,10 @@ describe("maybeInstallDaemon", () => {
|
||||
new Error("systemctl --user unavailable: Failed to connect to bus: No medium found"),
|
||||
);
|
||||
|
||||
await expect(
|
||||
maybeInstallDaemon({
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
port: 18789,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
await maybeInstallDaemon({
|
||||
runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() },
|
||||
port: 18789,
|
||||
});
|
||||
|
||||
expect(serviceInstall).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
@@ -17,12 +17,14 @@ import { resolveGatewayInstallToken } from "./gateway-install-token.js";
|
||||
import { guardCancel } from "./onboard-helpers.js";
|
||||
import { ensureSystemdUserLingerInteractive } from "./systemd-linger.js";
|
||||
|
||||
export type DaemonSetupOutcome = "succeeded" | "failed" | "skipped";
|
||||
|
||||
/** Prompt to install, reinstall, restart, or skip the local Gateway service. */
|
||||
export async function maybeInstallDaemon(params: {
|
||||
runtime: RuntimeEnv;
|
||||
port: number;
|
||||
daemonRuntime?: GatewayDaemonRuntime;
|
||||
}) {
|
||||
}): Promise<DaemonSetupOutcome> {
|
||||
const service = resolveGatewayService();
|
||||
let loaded;
|
||||
try {
|
||||
@@ -67,7 +69,7 @@ export async function maybeInstallDaemon(params: {
|
||||
shouldInstall = false;
|
||||
}
|
||||
if (action === "skip") {
|
||||
return;
|
||||
return "skipped";
|
||||
}
|
||||
if (action === "reinstall") {
|
||||
await withProgress(
|
||||
@@ -149,7 +151,7 @@ export async function maybeInstallDaemon(params: {
|
||||
if (installError) {
|
||||
note("Gateway service install failed: ".concat(installError), "Gateway");
|
||||
note(gatewayInstallErrorHint(), "Gateway");
|
||||
return;
|
||||
return "failed";
|
||||
}
|
||||
shouldCheckLinger = true;
|
||||
}
|
||||
@@ -166,4 +168,5 @@ export async function maybeInstallDaemon(params: {
|
||||
requireConfirm: true,
|
||||
});
|
||||
}
|
||||
return "succeeded";
|
||||
}
|
||||
|
||||
@@ -0,0 +1,151 @@
|
||||
// Configure wizard persistence tests protect config writes before local side effects.
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
intro: vi.fn(),
|
||||
outro: vi.fn(),
|
||||
select: vi.fn(),
|
||||
text: vi.fn(),
|
||||
note: vi.fn(),
|
||||
readConfigFileSnapshotForWrite: vi.fn(),
|
||||
writeWizardConfigFile: vi.fn(),
|
||||
probeGatewayReachable: vi.fn(),
|
||||
waitForGatewayReachable: vi.fn(),
|
||||
healthCommand: vi.fn(),
|
||||
maybeInstallDaemon: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./configure.shared.js", () => ({
|
||||
CONFIGURE_SECTION_OPTIONS: [
|
||||
{ value: "daemon", label: "Daemon", hint: "Manage the background service" },
|
||||
{ value: "health", label: "Health check", hint: "Run gateway checks" },
|
||||
],
|
||||
confirm: vi.fn(),
|
||||
intro: mocks.intro,
|
||||
outro: mocks.outro,
|
||||
select: mocks.select,
|
||||
text: mocks.text,
|
||||
}));
|
||||
|
||||
vi.mock("../config/config.js", () => ({
|
||||
readConfigFileSnapshotForWrite: mocks.readConfigFileSnapshotForWrite,
|
||||
resolveGatewayPort: () => 18789,
|
||||
}));
|
||||
|
||||
vi.mock("../config/logging.js", () => ({ logConfigUpdated: vi.fn() }));
|
||||
vi.mock("../../packages/terminal-core/src/note.js", () => ({ note: mocks.note }));
|
||||
vi.mock("../wizard/clack-prompter.js", () => ({ createClackPrompter: () => ({}) }));
|
||||
vi.mock("../wizard/setup.shared.js", () => ({
|
||||
writeWizardConfigFile: mocks.writeWizardConfigFile,
|
||||
}));
|
||||
vi.mock("../wizard/setup.secret-input.js", () => ({
|
||||
resolveSetupSecretInputString: vi.fn(async () => undefined),
|
||||
}));
|
||||
|
||||
vi.mock("./onboard-helpers.js", () => ({
|
||||
DEFAULT_WORKSPACE: "/tmp/openclaw-workspace",
|
||||
applyWizardMetadata: (config: OpenClawConfig) => config,
|
||||
guardCancel: (value: unknown) => value,
|
||||
probeGatewayReachable: mocks.probeGatewayReachable,
|
||||
resolveAdvertisedControlUiLinks: vi.fn(async () => ({
|
||||
httpUrl: "http://127.0.0.1:18789/",
|
||||
wsUrl: "ws://127.0.0.1:18789",
|
||||
})),
|
||||
resolveLocalControlUiProbeLinks: vi.fn(() => ({
|
||||
httpUrl: "http://127.0.0.1:18789/",
|
||||
wsUrl: "ws://127.0.0.1:18789",
|
||||
})),
|
||||
summarizeExistingConfig: vi.fn(() => "Gateway: remote"),
|
||||
waitForGatewayReachable: mocks.waitForGatewayReachable,
|
||||
}));
|
||||
|
||||
vi.mock("./onboard-agent-target.js", () => ({
|
||||
ensureOnboardingAgentWorkspace: vi.fn(),
|
||||
resolveOnboardingAgentTarget: () => ({
|
||||
agentId: "main",
|
||||
agentDir: "/tmp/openclaw-agent",
|
||||
workspaceDir: "/tmp/openclaw-workspace",
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../plugins/install-record-commit.js", () => ({
|
||||
commitConfigWithPendingPluginInstalls: vi.fn(),
|
||||
}));
|
||||
vi.mock("../plugins/plugin-registry.js", () => ({ resolvePluginContributionOwners: vi.fn() }));
|
||||
vi.mock("./configure.channels.js", () => ({ removeChannelConfigWizard: vi.fn() }));
|
||||
vi.mock("./configure.daemon.js", () => ({ maybeInstallDaemon: mocks.maybeInstallDaemon }));
|
||||
vi.mock("./configure.gateway-auth.js", () => ({ promptAuthConfig: vi.fn() }));
|
||||
vi.mock("./configure.gateway.js", () => ({ promptGatewayConfig: vi.fn() }));
|
||||
vi.mock("./health.js", () => ({ healthCommand: mocks.healthCommand }));
|
||||
vi.mock("./onboard-channels.js", () => ({ setupChannels: vi.fn() }));
|
||||
vi.mock("./onboard-remote.js", () => ({ promptRemoteGatewayConfig: vi.fn() }));
|
||||
vi.mock("./onboard-skills.js", () => ({ setupSkills: vi.fn() }));
|
||||
|
||||
import { runConfigureWizard } from "./configure.wizard.js";
|
||||
|
||||
const runtime = {
|
||||
log: vi.fn(),
|
||||
error: vi.fn(),
|
||||
exit: vi.fn(),
|
||||
};
|
||||
|
||||
describe("configure wizard persistence before local side effects", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mocks.readConfigFileSnapshotForWrite.mockResolvedValue({
|
||||
snapshot: {
|
||||
exists: true,
|
||||
valid: true,
|
||||
config: { gateway: { mode: "remote" } },
|
||||
issues: [],
|
||||
},
|
||||
writeOptions: {
|
||||
expectedConfigPath: "/tmp/openclaw.json",
|
||||
ownedConfigPathForWrite: "/tmp/openclaw.json",
|
||||
},
|
||||
});
|
||||
mocks.probeGatewayReachable.mockResolvedValue({ ok: false });
|
||||
mocks.text.mockResolvedValue("18789");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["health", "succeeded"],
|
||||
["daemon", "succeeded"],
|
||||
["health", "failed"],
|
||||
["daemon", "failed"],
|
||||
] as const)("persists Local before %s reports %s", async (section, outcome) => {
|
||||
const choices = ["local", section, "__continue"];
|
||||
const events: string[] = [];
|
||||
const writes: OpenClawConfig[] = [];
|
||||
mocks.select.mockImplementation(async () => choices.shift());
|
||||
mocks.writeWizardConfigFile.mockImplementation(async (config: OpenClawConfig) => {
|
||||
events.push("commit");
|
||||
writes.push(config);
|
||||
return config;
|
||||
});
|
||||
if (section === "health") {
|
||||
mocks.waitForGatewayReachable.mockImplementationOnce(async () => {
|
||||
events.push("health");
|
||||
return { ok: outcome === "succeeded" };
|
||||
});
|
||||
} else {
|
||||
mocks.maybeInstallDaemon.mockImplementationOnce(async () => {
|
||||
events.push("daemon");
|
||||
return outcome;
|
||||
});
|
||||
}
|
||||
|
||||
await runConfigureWizard({ command: "configure" }, runtime);
|
||||
|
||||
expect(writes).toHaveLength(1);
|
||||
expect(writes[0]?.gateway?.mode).toBe("local");
|
||||
expect(events).toEqual(["commit", section]);
|
||||
if (outcome === "failed") {
|
||||
expect(mocks.outro).toHaveBeenLastCalledWith(
|
||||
`Configuration updated, but ${section === "health" ? "health check" : "daemon setup"} failed.`,
|
||||
);
|
||||
expect(mocks.outro).not.toHaveBeenCalledWith("Configuration updated.");
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -388,6 +388,7 @@ describe("runConfigureWizard", () => {
|
||||
});
|
||||
vi.mocked(maybeInstallDaemon).mockImplementationOnce(async () => {
|
||||
events.push("daemon");
|
||||
return "succeeded";
|
||||
});
|
||||
|
||||
await runConfigureWizard(
|
||||
|
||||
@@ -25,7 +25,7 @@ import { WizardCancelledError } from "../wizard/prompts.js";
|
||||
import { resolveSetupSecretInputString } from "../wizard/setup.secret-input.js";
|
||||
import { writeWizardConfigFile } from "../wizard/setup.shared.js";
|
||||
import { removeChannelConfigWizard } from "./configure.channels.js";
|
||||
import { maybeInstallDaemon } from "./configure.daemon.js";
|
||||
import { maybeInstallDaemon, type DaemonSetupOutcome } from "./configure.daemon.js";
|
||||
import { promptAuthConfig } from "./configure.gateway-auth.js";
|
||||
import { promptGatewayConfig } from "./configure.gateway.js";
|
||||
import type {
|
||||
@@ -600,8 +600,8 @@ export async function runConfigureWizard(
|
||||
|
||||
let nextConfig = { ...baseConfig };
|
||||
let mergeBaseConfig = structuredClone(baseConfig);
|
||||
let didSetGatewayMode = false;
|
||||
if (shouldPromptGatewayRunMode && nextConfig.gateway?.mode !== "local") {
|
||||
let hasPendingConfig = shouldPromptGatewayRunMode && nextConfig.gateway?.mode !== "local";
|
||||
if (hasPendingConfig) {
|
||||
nextConfig = {
|
||||
...nextConfig,
|
||||
gateway: {
|
||||
@@ -609,7 +609,6 @@ export async function runConfigureWizard(
|
||||
mode: "local",
|
||||
},
|
||||
};
|
||||
didSetGatewayMode = true;
|
||||
}
|
||||
// Configure keeps legacy default-owner semantics; only explicit fleets opt into
|
||||
// the System Agent target used unconditionally by setup and recovery callers.
|
||||
@@ -619,8 +618,14 @@ export async function runConfigureWizard(
|
||||
: resolveOnboardingAgentTarget(inheritLegacyDefaultAgentId(baseConfig, nextConfig));
|
||||
let workspaceDir = resolveSetupTarget().workspaceDir;
|
||||
let gatewayPort = resolveGatewayPort(baseConfig);
|
||||
let didPersistConfig = false;
|
||||
let daemonSetupOutcome: DaemonSetupOutcome | undefined;
|
||||
let healthCheckOutcome: GatewayHealthCheckOutcome | undefined;
|
||||
|
||||
const persistConfig = async () => {
|
||||
const persistPendingConfig = async () => {
|
||||
if (!hasPendingConfig) {
|
||||
return;
|
||||
}
|
||||
nextConfig = applyWizardMetadata(nextConfig, {
|
||||
command: opts.command,
|
||||
mode: metadataMode,
|
||||
@@ -631,6 +636,8 @@ export async function runConfigureWizard(
|
||||
writeOptions: configWriteOwnership,
|
||||
});
|
||||
mergeBaseConfig = structuredClone(nextConfig);
|
||||
hasPendingConfig = false;
|
||||
didPersistConfig = true;
|
||||
logConfigUpdated(runtime);
|
||||
};
|
||||
|
||||
@@ -781,10 +788,14 @@ export async function runConfigureWizard(
|
||||
if (!didConfigureGateway) {
|
||||
await promptDaemonPort();
|
||||
}
|
||||
await maybeInstallDaemon({ runtime, port: gatewayPort });
|
||||
daemonSetupOutcome = await maybeInstallDaemon({ runtime, port: gatewayPort });
|
||||
},
|
||||
health: async () => {
|
||||
await runGatewayHealthCheck({ cfg: nextConfig, runtime, port: gatewayPort });
|
||||
healthCheckOutcome = await runGatewayHealthCheck({
|
||||
cfg: nextConfig,
|
||||
runtime,
|
||||
port: gatewayPort,
|
||||
});
|
||||
},
|
||||
} satisfies Record<WizardSection, () => Promise<void>>;
|
||||
|
||||
@@ -807,10 +818,11 @@ export async function runConfigureWizard(
|
||||
] as const) {
|
||||
if (selectedSections.includes(section)) {
|
||||
await sectionActions[section]();
|
||||
hasPendingConfig = true;
|
||||
}
|
||||
}
|
||||
|
||||
await persistConfig();
|
||||
await persistPendingConfig();
|
||||
|
||||
for (const section of ["daemon", "health"] as const) {
|
||||
if (selectedSections.includes(section)) {
|
||||
@@ -826,16 +838,20 @@ export async function runConfigureWizard(
|
||||
break;
|
||||
}
|
||||
ranSection = true;
|
||||
if (choice === "daemon" || choice === "health") {
|
||||
await persistPendingConfig();
|
||||
}
|
||||
await sectionActions[choice]();
|
||||
if (choice !== "daemon" && choice !== "health") {
|
||||
// Interactive setup commits each section before showing another prompt.
|
||||
await persistConfig();
|
||||
hasPendingConfig = true;
|
||||
await persistPendingConfig();
|
||||
}
|
||||
}
|
||||
|
||||
if (!ranSection) {
|
||||
if (didSetGatewayMode) {
|
||||
await persistConfig();
|
||||
if (hasPendingConfig) {
|
||||
await persistPendingConfig();
|
||||
outro("Gateway mode set to local.");
|
||||
return;
|
||||
}
|
||||
@@ -844,6 +860,21 @@ export async function runConfigureWizard(
|
||||
}
|
||||
}
|
||||
|
||||
const failedSideEffects = [
|
||||
...(daemonSetupOutcome === "failed" ? ["daemon setup"] : []),
|
||||
...(healthCheckOutcome === "failed" ? ["health check"] : []),
|
||||
];
|
||||
let completionMessage = didPersistConfig
|
||||
? "Configuration updated."
|
||||
: "No configuration changes selected.";
|
||||
if (failedSideEffects.length > 0) {
|
||||
completionMessage = `${didPersistConfig ? "Configuration updated" : "Configuration unchanged"}, but ${failedSideEffects.join(" and ")} failed.`;
|
||||
} else if (!didPersistConfig && healthCheckOutcome) {
|
||||
completionMessage = `Health check ${healthCheckOutcome === "succeeded" ? "completed" : "skipped"}.`;
|
||||
} else if (!didPersistConfig && daemonSetupOutcome) {
|
||||
completionMessage = `Daemon setup ${daemonSetupOutcome === "succeeded" ? "completed" : "skipped"}.`;
|
||||
}
|
||||
|
||||
if (shouldSkipGatewaySummary) {
|
||||
const remoteUrl = normalizeOptionalString(nextConfig.gateway?.remote?.url);
|
||||
if (remoteUrl) {
|
||||
@@ -854,7 +885,7 @@ export async function runConfigureWizard(
|
||||
"Gateway",
|
||||
);
|
||||
}
|
||||
outro("Configuration updated.");
|
||||
outro(completionMessage);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -923,7 +954,7 @@ export async function runConfigureWizard(
|
||||
"Control UI",
|
||||
);
|
||||
|
||||
outro("Configuration updated.");
|
||||
outro(completionMessage);
|
||||
} catch (err) {
|
||||
if (err instanceof WizardCancelledError) {
|
||||
runtime.exit(1);
|
||||
|
||||
Reference in New Issue
Block a user