From 85722d4cf210d4471d2bb9e4dd759ff3e2b70593 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 21 Mar 2026 16:59:22 -0700 Subject: [PATCH] refactor(doctor): extract legacy and unknown-key steps (#51938) --- src/commands/doctor-config-flow.ts | 80 ++++++++---------- .../doctor/shared/config-flow-steps.test.ts | 54 ++++++++++++ .../doctor/shared/config-flow-steps.ts | 84 +++++++++++++++++++ 3 files changed, 173 insertions(+), 45 deletions(-) create mode 100644 src/commands/doctor/shared/config-flow-steps.test.ts create mode 100644 src/commands/doctor/shared/config-flow-steps.ts diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 846776a996d..5b19e6ffe08 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -1,13 +1,12 @@ import { formatCliCommand } from "../cli/command-format.js"; import type { OpenClawConfig } from "../config/config.js"; -import { CONFIG_PATH, migrateLegacyConfig } from "../config/config.js"; -import { formatConfigIssueLines } from "../config/issue-format.js"; +import { CONFIG_PATH } from "../config/config.js"; import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js"; import { detectLegacyMatrixCrypto } from "../infra/matrix-legacy-crypto.js"; import { detectLegacyMatrixState } from "../infra/matrix-legacy-state.js"; import { sanitizeForLog } from "../terminal/ansi.js"; import { note } from "../terminal/note.js"; -import { noteOpencodeProviderOverrides, stripUnknownConfigKeys } from "./doctor-config-analysis.js"; +import { noteOpencodeProviderOverrides } from "./doctor-config-analysis.js"; import { runDoctorConfigPreflight } from "./doctor-config-preflight.js"; import { normalizeCompatibilityConfigValues } from "./doctor-legacy-config.js"; import type { DoctorOptions } from "./doctor-prompter.js"; @@ -29,6 +28,10 @@ import { scanTelegramAllowFromUsernameEntries, } from "./doctor/providers/telegram.js"; import { maybeRepairAllowlistPolicyAllowFrom } from "./doctor/shared/allowlist-policy-repair.js"; +import { + applyLegacyCompatibilityStep, + applyUnknownConfigKeyStep, +} from "./doctor/shared/config-flow-steps.js"; import { applyDoctorConfigMutation } from "./doctor/shared/config-mutation-state.js"; import { collectMissingDefaultAccountBindingWarnings, @@ -69,30 +72,20 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { let pendingChanges = false; let shouldWriteConfig = false; let fixHints: string[] = []; + const doctorFixCommand = formatCliCommand("openclaw doctor --fix"); - if (snapshot.legacyIssues.length > 0) { - note( - formatConfigIssueLines(snapshot.legacyIssues, "-").join("\n"), - "Compatibility config keys detected", - ); - const { config: migrated, changes } = migrateLegacyConfig(snapshot.parsed); - if (changes.length > 0) { - note(changes.join("\n"), "Doctor changes"); - } - if (migrated) { - candidate = migrated; - pendingChanges = pendingChanges || changes.length > 0; - } - if (shouldRepair) { - // Compatibility migration (2026-01-02, commit: 16420e5b) — normalize per-provider allowlists; move WhatsApp gating into channels.whatsapp.allowFrom. - if (migrated) { - cfg = migrated; - } - } else { - fixHints.push( - `Run "${formatCliCommand("openclaw doctor --fix")}" to apply compatibility migrations.`, - ); - } + const legacyStep = applyLegacyCompatibilityStep({ + snapshot, + state: { cfg, candidate, pendingChanges, fixHints }, + shouldRepair, + doctorFixCommand, + }); + ({ cfg, candidate, pendingChanges, fixHints } = legacyStep.state); + if (legacyStep.issueLines.length > 0) { + note(legacyStep.issueLines.join("\n"), "Compatibility config keys detected"); + } + if (legacyStep.changeLines.length > 0) { + note(legacyStep.changeLines.join("\n"), "Doctor changes"); } const normalized = normalizeCompatibilityConfigValues(candidate); @@ -102,7 +95,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { state: { cfg, candidate, pendingChanges, fixHints }, mutation: normalized, shouldRepair, - fixHint: `Run "${formatCliCommand("openclaw doctor --fix")}" to apply these changes.`, + fixHint: `Run "${doctorFixCommand}" to apply these changes.`, })); } @@ -113,7 +106,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { state: { cfg, candidate, pendingChanges, fixHints }, mutation: autoEnable, shouldRepair, - fixHint: `Run "${formatCliCommand("openclaw doctor --fix")}" to apply these changes.`, + fixHint: `Run "${doctorFixCommand}" to apply these changes.`, })); } @@ -250,7 +243,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { note( collectTelegramAllowFromUsernameWarnings({ hits, - doctorFixCommand: formatCliCommand("openclaw doctor --fix"), + doctorFixCommand, }).join("\n"), "Doctor warnings", ); @@ -261,7 +254,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { note( collectDiscordNumericIdWarnings({ hits: discordHits, - doctorFixCommand: formatCliCommand("openclaw doctor --fix"), + doctorFixCommand, }).join("\n"), "Doctor warnings", ); @@ -272,7 +265,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { note( collectOpenPolicyAllowFromWarnings({ changes: allowFromScan.changes, - doctorFixCommand: formatCliCommand("openclaw doctor --fix"), + doctorFixCommand, }).join("\n"), "Doctor warnings", ); @@ -294,7 +287,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { note( collectLegacyToolsBySenderWarnings({ hits: toolsBySenderHits, - doctorFixCommand: formatCliCommand("openclaw doctor --fix"), + doctorFixCommand, }).join("\n"), "Doctor warnings", ); @@ -305,7 +298,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { note( collectExecSafeBinCoverageWarnings({ hits: safeBinCoverage, - doctorFixCommand: formatCliCommand("openclaw doctor --fix"), + doctorFixCommand, }).join("\n"), "Doctor warnings", ); @@ -325,18 +318,15 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { note(collectMutableAllowlistWarnings(mutableAllowlistHits).join("\n"), "Doctor warnings"); } - const unknown = stripUnknownConfigKeys(candidate); - if (unknown.removed.length > 0) { - const lines = unknown.removed.map((path) => `- ${path}`).join("\n"); - candidate = unknown.config; - pendingChanges = true; - if (shouldRepair) { - cfg = unknown.config; - note(lines, "Doctor changes"); - } else { - note(lines, "Unknown config keys"); - fixHints.push('Run "openclaw doctor --fix" to remove these keys.'); - } + const unknownStep = applyUnknownConfigKeyStep({ + state: { cfg, candidate, pendingChanges, fixHints }, + shouldRepair, + doctorFixCommand, + }); + ({ cfg, candidate, pendingChanges, fixHints } = unknownStep.state); + if (unknownStep.removed.length > 0) { + const lines = unknownStep.removed.map((path) => `- ${path}`).join("\n"); + note(lines, shouldRepair ? "Doctor changes" : "Unknown config keys"); } if (!shouldRepair && pendingChanges) { diff --git a/src/commands/doctor/shared/config-flow-steps.test.ts b/src/commands/doctor/shared/config-flow-steps.test.ts new file mode 100644 index 00000000000..3383076e589 --- /dev/null +++ b/src/commands/doctor/shared/config-flow-steps.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../../config/config.js"; +import type { DoctorConfigPreflightResult } from "../../doctor-config-preflight.js"; +import { applyLegacyCompatibilityStep, applyUnknownConfigKeyStep } from "./config-flow-steps.js"; + +describe("doctor config flow steps", () => { + it("collects legacy compatibility issue lines and preview fix hints", () => { + const result = applyLegacyCompatibilityStep({ + snapshot: { + exists: true, + parsed: { heartbeat: { enabled: true } }, + legacyIssues: [{ path: "heartbeat", message: "use agents.defaults.heartbeat" }], + path: "/tmp/config.json", + valid: true, + issues: [], + raw: "{}", + resolved: {}, + config: {}, + warnings: [], + } satisfies DoctorConfigPreflightResult["snapshot"], + state: { + cfg: {}, + candidate: {}, + pendingChanges: false, + fixHints: [], + }, + shouldRepair: false, + doctorFixCommand: "openclaw doctor --fix", + }); + + expect(result.issueLines).toEqual([expect.stringContaining("- heartbeat:")]); + expect(result.changeLines).not.toEqual([]); + expect(result.state.fixHints).toContain( + 'Run "openclaw doctor --fix" to apply compatibility migrations.', + ); + }); + + it("removes unknown keys and adds preview hint", () => { + const result = applyUnknownConfigKeyStep({ + state: { + cfg: {}, + candidate: { bogus: true } as unknown as OpenClawConfig, + pendingChanges: false, + fixHints: [], + }, + shouldRepair: false, + doctorFixCommand: "openclaw doctor --fix", + }); + + expect(result.removed).toEqual(["bogus"]); + expect(result.state.candidate).toEqual({}); + expect(result.state.fixHints).toContain('Run "openclaw doctor --fix" to remove these keys.'); + }); +}); diff --git a/src/commands/doctor/shared/config-flow-steps.ts b/src/commands/doctor/shared/config-flow-steps.ts new file mode 100644 index 00000000000..7c78dd89404 --- /dev/null +++ b/src/commands/doctor/shared/config-flow-steps.ts @@ -0,0 +1,84 @@ +import { migrateLegacyConfig } from "../../../config/config.js"; +import { formatConfigIssueLines } from "../../../config/issue-format.js"; +import { stripUnknownConfigKeys } from "../../doctor-config-analysis.js"; +import type { DoctorConfigPreflightResult } from "../../doctor-config-preflight.js"; +import type { DoctorConfigMutationState } from "./config-mutation-state.js"; + +export function applyLegacyCompatibilityStep(params: { + snapshot: DoctorConfigPreflightResult["snapshot"]; + state: DoctorConfigMutationState; + shouldRepair: boolean; + doctorFixCommand: string; +}): { + state: DoctorConfigMutationState; + issueLines: string[]; + changeLines: string[]; +} { + if (params.snapshot.legacyIssues.length === 0) { + return { + state: params.state, + issueLines: [], + changeLines: [], + }; + } + + const issueLines = formatConfigIssueLines(params.snapshot.legacyIssues, "-"); + const { config: migrated, changes } = migrateLegacyConfig(params.snapshot.parsed); + if (!migrated) { + return { + state: { + ...params.state, + fixHints: params.shouldRepair + ? params.state.fixHints + : [ + ...params.state.fixHints, + `Run "${params.doctorFixCommand}" to apply compatibility migrations.`, + ], + }, + issueLines, + changeLines: changes, + }; + } + + return { + state: { + cfg: params.shouldRepair ? migrated : params.state.cfg, + candidate: migrated, + pendingChanges: params.state.pendingChanges || changes.length > 0, + fixHints: params.shouldRepair + ? params.state.fixHints + : [ + ...params.state.fixHints, + `Run "${params.doctorFixCommand}" to apply compatibility migrations.`, + ], + }, + issueLines, + changeLines: changes, + }; +} + +export function applyUnknownConfigKeyStep(params: { + state: DoctorConfigMutationState; + shouldRepair: boolean; + doctorFixCommand: string; +}): { + state: DoctorConfigMutationState; + removed: string[]; +} { + const unknown = stripUnknownConfigKeys(params.state.candidate); + if (unknown.removed.length === 0) { + return { state: params.state, removed: [] }; + } + + return { + state: { + cfg: params.shouldRepair ? unknown.config : params.state.cfg, + candidate: unknown.config, + pendingChanges: true, + fixHints: params.shouldRepair + ? params.state.fixHints + : [...params.state.fixHints, `Run "${params.doctorFixCommand}" to remove these keys.`], + }, + removed: unknown.removed, + }; +}