From 865a90ccabf7c2f79e8967783d5f641a2700a836 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 21 Mar 2026 16:45:33 -0700 Subject: [PATCH] refactor(doctor): extract config mutation state helper (#51935) --- src/commands/doctor-config-flow.ts | 77 +++++++++++-------- .../shared/config-mutation-state.test.ts | 69 +++++++++++++++++ .../doctor/shared/config-mutation-state.ts | 34 ++++++++ 3 files changed, 147 insertions(+), 33 deletions(-) create mode 100644 src/commands/doctor/shared/config-mutation-state.test.ts create mode 100644 src/commands/doctor/shared/config-mutation-state.ts diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 46cc7c9b8d1..846776a996d 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -29,6 +29,7 @@ import { scanTelegramAllowFromUsernameEntries, } from "./doctor/providers/telegram.js"; import { maybeRepairAllowlistPolicyAllowFrom } from "./doctor/shared/allowlist-policy-repair.js"; +import { applyDoctorConfigMutation } from "./doctor/shared/config-mutation-state.js"; import { collectMissingDefaultAccountBindingWarnings, collectMissingExplicitDefaultAccountWarnings, @@ -67,7 +68,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { let candidate = structuredClone(baseCfg); let pendingChanges = false; let shouldWriteConfig = false; - const fixHints: string[] = []; + let fixHints: string[] = []; if (snapshot.legacyIssues.length > 0) { note( @@ -97,25 +98,23 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { const normalized = normalizeCompatibilityConfigValues(candidate); if (normalized.changes.length > 0) { note(normalized.changes.join("\n"), "Doctor changes"); - candidate = normalized.config; - pendingChanges = true; - if (shouldRepair) { - cfg = normalized.config; - } else { - fixHints.push(`Run "${formatCliCommand("openclaw doctor --fix")}" to apply these changes.`); - } + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: normalized, + shouldRepair, + fixHint: `Run "${formatCliCommand("openclaw doctor --fix")}" to apply these changes.`, + })); } const autoEnable = applyPluginAutoEnable({ config: candidate, env: process.env }); if (autoEnable.changes.length > 0) { note(autoEnable.changes.join("\n"), "Doctor changes"); - candidate = autoEnable.config; - pendingChanges = true; - if (shouldRepair) { - cfg = autoEnable.config; - } else { - fixHints.push(`Run "${formatCliCommand("openclaw doctor --fix")}" to apply these changes.`); - } + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: autoEnable, + shouldRepair, + fixHint: `Run "${formatCliCommand("openclaw doctor --fix")}" to apply these changes.`, + })); } const matrixLegacyState = detectLegacyMatrixState({ @@ -172,17 +171,21 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { const repair = await maybeRepairTelegramAllowFromUsernames(candidate); if (repair.changes.length > 0) { note(repair.changes.join("\n"), "Doctor changes"); - candidate = repair.config; - pendingChanges = true; - cfg = repair.config; + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: repair, + shouldRepair, + })); } const discordRepair = maybeRepairDiscordNumericIds(candidate); if (discordRepair.changes.length > 0) { note(discordRepair.changes.join("\n"), "Doctor changes"); - candidate = discordRepair.config; - pendingChanges = true; - cfg = discordRepair.config; + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: discordRepair, + shouldRepair, + })); } const allowFromRepair = maybeRepairOpenPolicyAllowFrom(candidate); @@ -191,17 +194,21 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { allowFromRepair.changes.map((line) => sanitizeForLog(line)).join("\n"), "Doctor changes", ); - candidate = allowFromRepair.config; - pendingChanges = true; - cfg = allowFromRepair.config; + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: allowFromRepair, + shouldRepair, + })); } const allowlistRepair = await maybeRepairAllowlistPolicyAllowFrom(candidate); if (allowlistRepair.changes.length > 0) { note(allowlistRepair.changes.join("\n"), "Doctor changes"); - candidate = allowlistRepair.config; - pendingChanges = true; - cfg = allowlistRepair.config; + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: allowlistRepair, + shouldRepair, + })); } const emptyAllowlistWarnings = scanEmptyAllowlistPolicyWarnings(candidate, { @@ -218,17 +225,21 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { const toolsBySenderRepair = maybeRepairLegacyToolsBySenderKeys(candidate); if (toolsBySenderRepair.changes.length > 0) { note(toolsBySenderRepair.changes.join("\n"), "Doctor changes"); - candidate = toolsBySenderRepair.config; - pendingChanges = true; - cfg = toolsBySenderRepair.config; + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: toolsBySenderRepair, + shouldRepair, + })); } const safeBinProfileRepair = maybeRepairExecSafeBinProfiles(candidate); if (safeBinProfileRepair.changes.length > 0) { note(safeBinProfileRepair.changes.join("\n"), "Doctor changes"); - candidate = safeBinProfileRepair.config; - pendingChanges = true; - cfg = safeBinProfileRepair.config; + ({ cfg, candidate, pendingChanges, fixHints } = applyDoctorConfigMutation({ + state: { cfg, candidate, pendingChanges, fixHints }, + mutation: safeBinProfileRepair, + shouldRepair, + })); } if (safeBinProfileRepair.warnings.length > 0) { note(safeBinProfileRepair.warnings.join("\n"), "Doctor warnings"); diff --git a/src/commands/doctor/shared/config-mutation-state.test.ts b/src/commands/doctor/shared/config-mutation-state.test.ts new file mode 100644 index 00000000000..b27063b0a15 --- /dev/null +++ b/src/commands/doctor/shared/config-mutation-state.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; +import { applyDoctorConfigMutation } from "./config-mutation-state.js"; + +describe("doctor config mutation state", () => { + it("updates candidate and fix hints in preview mode", () => { + const next = applyDoctorConfigMutation({ + state: { + cfg: { channels: {} }, + candidate: { channels: {} }, + pendingChanges: false, + fixHints: [], + }, + mutation: { + config: { channels: { signal: { enabled: true } } }, + changes: ["enabled signal"], + }, + shouldRepair: false, + fixHint: 'Run "openclaw doctor --fix" to apply these changes.', + }); + + expect(next).toEqual({ + cfg: { channels: {} }, + candidate: { channels: { signal: { enabled: true } } }, + pendingChanges: true, + fixHints: ['Run "openclaw doctor --fix" to apply these changes.'], + }); + }); + + it("updates cfg directly in repair mode", () => { + const next = applyDoctorConfigMutation({ + state: { + cfg: { channels: {} }, + candidate: { channels: {} }, + pendingChanges: false, + fixHints: [], + }, + mutation: { + config: { channels: { signal: { enabled: true } } }, + changes: ["enabled signal"], + }, + shouldRepair: true, + fixHint: 'Run "openclaw doctor --fix" to apply these changes.', + }); + + expect(next).toEqual({ + cfg: { channels: { signal: { enabled: true } } }, + candidate: { channels: { signal: { enabled: true } } }, + pendingChanges: true, + fixHints: [], + }); + }); + + it("stays unchanged when there are no changes", () => { + const state = { + cfg: { channels: {} }, + candidate: { channels: {} }, + pendingChanges: false, + fixHints: [], + }; + + expect( + applyDoctorConfigMutation({ + state, + mutation: { config: { channels: { signal: { enabled: true } } }, changes: [] }, + shouldRepair: false, + }), + ).toBe(state); + }); +}); diff --git a/src/commands/doctor/shared/config-mutation-state.ts b/src/commands/doctor/shared/config-mutation-state.ts new file mode 100644 index 00000000000..d08ad5cde9b --- /dev/null +++ b/src/commands/doctor/shared/config-mutation-state.ts @@ -0,0 +1,34 @@ +import type { OpenClawConfig } from "../../../config/config.js"; + +export type DoctorConfigMutationState = { + cfg: OpenClawConfig; + candidate: OpenClawConfig; + pendingChanges: boolean; + fixHints: string[]; +}; + +export type DoctorConfigMutationResult = { + config: OpenClawConfig; + changes: string[]; +}; + +export function applyDoctorConfigMutation(params: { + state: DoctorConfigMutationState; + mutation: DoctorConfigMutationResult; + shouldRepair: boolean; + fixHint?: string; +}): DoctorConfigMutationState { + if (params.mutation.changes.length === 0) { + return params.state; + } + + return { + cfg: params.shouldRepair ? params.mutation.config : params.state.cfg, + candidate: params.mutation.config, + pendingChanges: true, + fixHints: + !params.shouldRepair && params.fixHint + ? [...params.state.fixHints, params.fixHint] + : params.state.fixHints, + }; +}