From 52008e2e60ab45252257385e4582ded2f189203a Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 3 Apr 2026 23:16:11 +0900 Subject: [PATCH] fix(doctor): clarify legacy config migration guidance (#60326) --- src/commands/doctor-config-flow.test.ts | 69 ++++++++++--------- src/commands/doctor-config-flow.ts | 2 +- .../doctor/providers/telegram.test.ts | 39 +++++++---- src/commands/doctor/providers/telegram.ts | 2 +- .../doctor/shared/config-flow-steps.test.ts | 2 +- .../doctor/shared/config-flow-steps.ts | 4 +- 6 files changed, 69 insertions(+), 49 deletions(-) diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index d4381848ea3..4655c6cfa84 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -35,28 +35,6 @@ async function collectDoctorWarnings(config: Record): Promise | undefined; - -async function loadFreshDoctorFlowDeps(): Promise { - if (!cachedDoctorFlowDeps) { - vi.resetModules(); - cachedDoctorFlowDeps = (async () => { - const freshNoteModule = await import("../terminal/note.js"); - const doctorFlowModule = await import("./doctor-config-flow.js"); - return { - noteModule: freshNoteModule, - loadAndMaybeMigrateDoctorConfig: doctorFlowModule.loadAndMaybeMigrateDoctorConfig, - }; - })(); - } - return await cachedDoctorFlowDeps; -} - type DiscordGuildRule = { users: string[]; roles: string[]; @@ -675,9 +653,7 @@ describe("doctor config flow", () => { }); it("sanitizes config-derived doctor warnings and changes before logging", async () => { - const { noteModule: freshNoteModule, loadAndMaybeMigrateDoctorConfig: loadDoctorFlowFresh } = - await loadFreshDoctorFlowDeps(); - const noteSpy = vi.spyOn(freshNoteModule, "note").mockImplementation(() => {}); + const noteSpy = vi.spyOn(noteModule, "note").mockImplementation(() => {}); try { await runDoctorConfigWithInput({ repair: true, @@ -710,7 +686,7 @@ describe("doctor config flow", () => { }, }, }, - run: loadDoctorFlowFresh, + run: loadAndMaybeMigrateDoctorConfig, }); const outputs = noteSpy.mock.calls @@ -745,9 +721,7 @@ describe("doctor config flow", () => { }); it("warns and continues when Telegram account inspection hits inactive SecretRef surfaces", async () => { - const { noteModule: freshNoteModule, loadAndMaybeMigrateDoctorConfig: loadDoctorFlowFresh } = - await loadFreshDoctorFlowDeps(); - const noteSpy = vi.spyOn(freshNoteModule, "note").mockImplementation(() => {}); + const noteSpy = vi.spyOn(noteModule, "note").mockImplementation(() => {}); const fetchSpy = vi.fn(); vi.stubGlobal("fetch", fetchSpy); try { @@ -771,7 +745,7 @@ describe("doctor config flow", () => { }, }, }, - run: loadDoctorFlowFresh, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as { @@ -791,7 +765,7 @@ describe("doctor config flow", () => { expect( noteSpy.mock.calls.some((call) => String(call[0]).includes( - "Telegram allowFrom contains @username entries, but no Telegram bot token is configured", + "Telegram allowFrom contains @username entries, but configured Telegram bot credentials are unavailable in this command path", ), ), ).toBe(true); @@ -1180,6 +1154,39 @@ describe("doctor config flow", () => { }); }); + it("warns clearly about legacy config keys and points to doctor --fix", async () => { + const noteSpy = vi.spyOn(noteModule, "note").mockImplementation(() => {}); + try { + await runDoctorConfigWithInput({ + config: { + heartbeat: { + model: "anthropic/claude-3-5-haiku-20241022", + every: "30m", + }, + }, + run: loadAndMaybeMigrateDoctorConfig, + }); + + expect( + noteSpy.mock.calls.some( + ([message, title]) => + title === "Legacy config keys detected" && + String(message).includes("heartbeat:") && + String(message).includes("agents.defaults.heartbeat"), + ), + ).toBe(true); + expect( + noteSpy.mock.calls.some( + ([message, title]) => + title === "Doctor" && + String(message).includes('Run "openclaw doctor --fix" to migrate legacy config keys.'), + ), + ).toBe(true); + } finally { + noteSpy.mockRestore(); + } + }); + it("migrates top-level heartbeat visibility into channels.defaults.heartbeat on repair", async () => { const result = await runDoctorConfigWithInput({ repair: true, diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index acbdcfc890d..33ce66d4b65 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -51,7 +51,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { }); ({ cfg, candidate, pendingChanges, fixHints } = legacyStep.state); if (legacyStep.issueLines.length > 0) { - note(legacyStep.issueLines.join("\n"), "Compatibility config keys detected"); + note(legacyStep.issueLines.join("\n"), "Legacy config keys detected"); } if (legacyStep.changeLines.length > 0) { note(legacyStep.changeLines.join("\n"), "Doctor changes"); diff --git a/src/commands/doctor/providers/telegram.test.ts b/src/commands/doctor/providers/telegram.test.ts index 8ce884bdd77..4bef704a797 100644 --- a/src/commands/doctor/providers/telegram.test.ts +++ b/src/commands/doctor/providers/telegram.test.ts @@ -27,16 +27,19 @@ vi.mock("../../../channels/plugins/registry.js", () => ({ getChannelPlugin: getChannelPluginMock, })); -import { - collectTelegramAllowFromUsernameWarnings, - collectTelegramEmptyAllowlistExtraWarnings, - collectTelegramGroupPolicyWarnings, - maybeRepairTelegramAllowFromUsernames, - scanTelegramAllowFromUsernameEntries, -} from "./telegram.js"; +type TelegramDoctorModule = typeof import("./telegram.js"); + +let telegramDoctorModule: Promise | undefined; + +async function loadTelegramDoctorModule(): Promise { + telegramDoctorModule ??= import("./telegram.js"); + return await telegramDoctorModule; +} describe("doctor telegram provider warnings", () => { beforeEach(() => { + vi.resetModules(); + telegramDoctorModule = undefined; resolveCommandSecretRefsViaGatewayMock.mockReset().mockImplementation(async ({ config }) => ({ resolvedConfig: config, diagnostics: [], @@ -64,7 +67,8 @@ describe("doctor telegram provider warnings", () => { }); }); - it("shows first-run guidance when groups are not configured yet", () => { + it("shows first-run guidance when groups are not configured yet", async () => { + const { collectTelegramGroupPolicyWarnings } = await loadTelegramDoctorModule(); const warnings = collectTelegramGroupPolicyWarnings({ account: { botToken: "123:abc", @@ -81,7 +85,8 @@ describe("doctor telegram provider warnings", () => { expect(warnings[0]).toContain("channels.telegram.groups"); }); - it("warns when configured groups still have no usable sender allowlist", () => { + it("warns when configured groups still have no usable sender allowlist", async () => { + const { collectTelegramGroupPolicyWarnings } = await loadTelegramDoctorModule(); const warnings = collectTelegramGroupPolicyWarnings({ account: { botToken: "123:abc", @@ -100,7 +105,8 @@ describe("doctor telegram provider warnings", () => { ]); }); - it("stays quiet when allowFrom can satisfy group allowlist mode", () => { + it("stays quiet when allowFrom can satisfy group allowlist mode", async () => { + const { collectTelegramGroupPolicyWarnings } = await loadTelegramDoctorModule(); const warnings = collectTelegramGroupPolicyWarnings({ account: { botToken: "123:abc", @@ -116,7 +122,8 @@ describe("doctor telegram provider warnings", () => { expect(warnings).toEqual([]); }); - it("returns extra empty-allowlist warnings only for telegram allowlist groups", () => { + it("returns extra empty-allowlist warnings only for telegram allowlist groups", async () => { + const { collectTelegramEmptyAllowlistExtraWarnings } = await loadTelegramDoctorModule(); const warnings = collectTelegramEmptyAllowlistExtraWarnings({ account: { botToken: "123:abc", @@ -143,7 +150,8 @@ describe("doctor telegram provider warnings", () => { ).toEqual([]); }); - it("finds non-numeric telegram allowFrom username entries across account scopes", () => { + it("finds non-numeric telegram allowFrom username entries across account scopes", async () => { + const { scanTelegramAllowFromUsernameEntries } = await loadTelegramDoctorModule(); const hits = scanTelegramAllowFromUsernameEntries({ channels: { telegram: { @@ -179,7 +187,8 @@ describe("doctor telegram provider warnings", () => { ]); }); - it("formats allowFrom username warnings", () => { + it("formats allowFrom username warnings", async () => { + const { collectTelegramAllowFromUsernameWarnings } = await loadTelegramDoctorModule(); const warnings = collectTelegramAllowFromUsernameWarnings({ hits: [{ path: "channels.telegram.allowFrom", entry: "@top" }], doctorFixCommand: "openclaw doctor --fix", @@ -192,6 +201,7 @@ describe("doctor telegram provider warnings", () => { }); it("repairs Telegram @username allowFrom entries to numeric ids", async () => { + const { maybeRepairTelegramAllowFromUsernames } = await loadTelegramDoctorModule(); telegramResolverMock.mockImplementation(async ({ inputs }: { inputs: string[] }) => { switch (inputs[0]?.toLowerCase()) { case "@testuser": @@ -247,6 +257,7 @@ describe("doctor telegram provider warnings", () => { }); it("sanitizes Telegram allowFrom repair change lines before logging", async () => { + const { maybeRepairTelegramAllowFromUsernames } = await loadTelegramDoctorModule(); telegramResolverMock.mockImplementation(async ({ inputs }: { inputs: string[] }) => { if (inputs[0] === "@\u001b[31mtestuser") { return [{ input: inputs[0], resolved: true, id: "12345" }]; @@ -273,6 +284,7 @@ describe("doctor telegram provider warnings", () => { }); it("keeps Telegram allowFrom entries unchanged when configured credentials are unavailable", async () => { + const { maybeRepairTelegramAllowFromUsernames } = await loadTelegramDoctorModule(); inspectTelegramAccountMock.mockImplementation(() => ({ enabled: true, token: "", @@ -311,6 +323,7 @@ describe("doctor telegram provider warnings", () => { }); it("uses network settings for Telegram allowFrom repair but ignores apiRoot and proxy", async () => { + const { maybeRepairTelegramAllowFromUsernames } = await loadTelegramDoctorModule(); resolveCommandSecretRefsViaGatewayMock.mockResolvedValue({ resolvedConfig: { channels: { diff --git a/src/commands/doctor/providers/telegram.ts b/src/commands/doctor/providers/telegram.ts index 175bfb022b3..924583823aa 100644 --- a/src/commands/doctor/providers/telegram.ts +++ b/src/commands/doctor/providers/telegram.ts @@ -152,7 +152,7 @@ export async function maybeRepairTelegramAllowFromUsernames(cfg: OpenClawConfig) }); const hasConfiguredUnavailableToken = listTelegramAccountIds(cfg).some((accountId) => { const inspected = inspectTelegramAccount({ cfg, accountId }); - return inspected.enabled && inspected.tokenStatus === "configured_unavailable"; + return inspected.tokenStatus === "configured_unavailable"; }); const tokenResolutionWarnings: string[] = []; const resolverAccountIds: string[] = []; diff --git a/src/commands/doctor/shared/config-flow-steps.test.ts b/src/commands/doctor/shared/config-flow-steps.test.ts index b6192d638fb..88c4e284697 100644 --- a/src/commands/doctor/shared/config-flow-steps.test.ts +++ b/src/commands/doctor/shared/config-flow-steps.test.ts @@ -33,7 +33,7 @@ describe("doctor config flow steps", () => { 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.', + 'Run "openclaw doctor --fix" to migrate legacy config keys.', ); }); diff --git a/src/commands/doctor/shared/config-flow-steps.ts b/src/commands/doctor/shared/config-flow-steps.ts index 7c78dd89404..6778f90170d 100644 --- a/src/commands/doctor/shared/config-flow-steps.ts +++ b/src/commands/doctor/shared/config-flow-steps.ts @@ -32,7 +32,7 @@ export function applyLegacyCompatibilityStep(params: { ? params.state.fixHints : [ ...params.state.fixHints, - `Run "${params.doctorFixCommand}" to apply compatibility migrations.`, + `Run "${params.doctorFixCommand}" to migrate legacy config keys.`, ], }, issueLines, @@ -49,7 +49,7 @@ export function applyLegacyCompatibilityStep(params: { ? params.state.fixHints : [ ...params.state.fixHints, - `Run "${params.doctorFixCommand}" to apply compatibility migrations.`, + `Run "${params.doctorFixCommand}" to migrate legacy config keys.`, ], }, issueLines,