diff --git a/extensions/anthropic/doctor-contract-api.ts b/extensions/anthropic/doctor-contract-api.ts new file mode 100644 index 00000000000..d4716ba7eb4 --- /dev/null +++ b/extensions/anthropic/doctor-contract-api.ts @@ -0,0 +1 @@ +export const legacyConfigRules = []; diff --git a/extensions/memory-wiki/doctor-contract-api.ts b/extensions/memory-wiki/doctor-contract-api.ts new file mode 100644 index 00000000000..db610ee157d --- /dev/null +++ b/extensions/memory-wiki/doctor-contract-api.ts @@ -0,0 +1 @@ +export { legacyConfigRules, normalizeCompatibilityConfig } from "./src/config-compat.js"; diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index a284b82116a..e5d95c1cb61 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -826,6 +826,26 @@ vi.mock("../plugins/doctor-contract-registry.js", () => { }; }); +vi.mock("./doctor/shared/legacy-config-issues.js", async () => { + const { + collectRelevantDoctorPluginIds, + listPluginDoctorLegacyConfigRules, + }: typeof import("../plugins/doctor-contract-registry.js") = + await import("../plugins/doctor-contract-registry.js"); + const { findLegacyConfigIssues }: typeof import("../config/legacy.js") = + await import("../config/legacy.js"); + return { + findDoctorLegacyConfigIssues: (raw: unknown, sourceRaw?: unknown) => + findLegacyConfigIssues( + raw, + sourceRaw, + listPluginDoctorLegacyConfigRules({ + pluginIds: collectRelevantDoctorPluginIds(raw), + }), + ), + }; +}); + vi.mock("../plugins/setup-registry.js", () => ({ resolvePluginSetupAutoEnableReasons: vi.fn(() => []), runPluginSetupConfigMigrations: vi.fn(({ config }: { config: unknown }) => ({ diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 9ffe53d2304..b89c5c54eec 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -1,6 +1,5 @@ import path from "node:path"; import { formatCliCommand } from "../cli/command-format.js"; -import { findLegacyConfigIssues } from "../config/legacy.js"; import { CONFIG_PATH } from "../config/paths.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { RuntimeEnv } from "../runtime.js"; @@ -92,15 +91,9 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { if (snapshot.parsed === snapshot.sourceConfig) { return []; } - const { collectRelevantDoctorPluginIds, listPluginDoctorLegacyConfigRules } = - await import("../plugins/doctor-contract-registry.js"); - return findLegacyConfigIssues( - snapshot.parsed, - snapshot.parsed, - listPluginDoctorLegacyConfigRules({ - pluginIds: collectRelevantDoctorPluginIds(snapshot.parsed), - }), - ); + const { findDoctorLegacyConfigIssues } = + await import("./doctor/shared/legacy-config-issues.js"); + return findDoctorLegacyConfigIssues(snapshot.parsed, snapshot.parsed); })(); const seenLegacyIssues = new Set( snapshot.legacyIssues.map((issue) => `${issue.path}:${issue.message}`), diff --git a/src/commands/doctor-config-preflight.ts b/src/commands/doctor-config-preflight.ts index aa74b2159b6..f653f70c375 100644 --- a/src/commands/doctor-config-preflight.ts +++ b/src/commands/doctor-config-preflight.ts @@ -2,16 +2,12 @@ import fs from "node:fs/promises"; import path from "node:path"; import { readConfigFileSnapshot, recoverConfigFromJsonRootSuffix } from "../config/io.js"; import { formatConfigIssueLines } from "../config/issue-format.js"; -import { findLegacyConfigIssues } from "../config/legacy.js"; import type { LegacyConfigIssue } from "../config/types.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { - collectRelevantDoctorPluginIds, - listPluginDoctorLegacyConfigRules, -} from "../plugins/doctor-contract-registry.js"; import { note } from "../terminal/note.js"; import { resolveHomeDir } from "../utils.js"; import { noteIncludeConfinementWarning } from "./doctor-config-analysis.js"; +import { findDoctorLegacyConfigIssues } from "./doctor/shared/legacy-config-issues.js"; async function maybeMigrateLegacyConfig(): Promise { const changes: string[] = []; @@ -69,13 +65,7 @@ function collectDoctorLegacyIssues( } const resolvedRaw = snapshot.sourceConfig ?? snapshot.config ?? {}; const sourceRaw = snapshot.parsed ?? resolvedRaw; - return findLegacyConfigIssues( - resolvedRaw, - sourceRaw, - listPluginDoctorLegacyConfigRules({ - pluginIds: collectRelevantDoctorPluginIds(resolvedRaw), - }), - ); + return findDoctorLegacyConfigIssues(resolvedRaw, sourceRaw); } function addDoctorLegacyIssues( diff --git a/src/commands/doctor/shared/legacy-config-issues.ts b/src/commands/doctor/shared/legacy-config-issues.ts new file mode 100644 index 00000000000..af53e9bdabb --- /dev/null +++ b/src/commands/doctor/shared/legacy-config-issues.ts @@ -0,0 +1,52 @@ +import { collectChannelLegacyConfigRules } from "../../../channels/plugins/legacy-config.js"; +import { findLegacyConfigIssues } from "../../../config/legacy.js"; +import type { LegacyConfigRule } from "../../../config/legacy.shared.js"; +import type { LegacyConfigIssue } from "../../../config/types.js"; +import { + collectRelevantDoctorPluginIds, + collectRelevantDoctorPluginIdsForTouchedPaths, + listPluginDoctorLegacyConfigRules, +} from "../../../plugins/doctor-contract-registry.js"; + +function collectConfiguredChannelIds(raw: unknown): ReadonlySet { + if (!raw || typeof raw !== "object") { + return new Set(); + } + const channels = (raw as { channels?: unknown }).channels; + if (!channels || typeof channels !== "object" || Array.isArray(channels)) { + return new Set(); + } + return new Set(Object.keys(channels).filter((channelId) => channelId !== "defaults")); +} + +function collectPluginLegacyConfigRules( + raw: unknown, + touchedPaths?: ReadonlyArray>, +): LegacyConfigRule[] { + const channelIds = collectConfiguredChannelIds(raw); + const pluginIds = ( + touchedPaths + ? collectRelevantDoctorPluginIdsForTouchedPaths({ raw, touchedPaths }) + : collectRelevantDoctorPluginIds(raw) + ).filter((pluginId) => !channelIds.has(pluginId)); + if (pluginIds.length === 0) { + return []; + } + return listPluginDoctorLegacyConfigRules({ pluginIds }); +} + +export function findDoctorLegacyConfigIssues( + raw: unknown, + sourceRaw?: unknown, + touchedPaths?: ReadonlyArray>, +): LegacyConfigIssue[] { + return findLegacyConfigIssues( + raw, + sourceRaw, + [ + ...collectChannelLegacyConfigRules(raw, touchedPaths), + ...collectPluginLegacyConfigRules(raw, touchedPaths), + ], + touchedPaths, + ); +} diff --git a/src/config/config-misc.test.ts b/src/config/config-misc.test.ts index b8daa3bf362..c7683dac92f 100644 --- a/src/config/config-misc.test.ts +++ b/src/config/config-misc.test.ts @@ -882,8 +882,8 @@ describe("config strict validation", () => { const snap = await readConfigFileSnapshot(); expect(snap.valid).toBe(false); - expect(snap.issues.some((issue) => issue.path === "memorySearch")).toBe(true); - expect(snap.legacyIssues).toEqual([]); + expect(snap.issues.some((issue) => issue.message.includes('"memorySearch"'))).toBe(true); + expect(snap.legacyIssues.some((issue) => issue.path === "memorySearch")).toBe(true); expect((snap.sourceConfig as { memorySearch?: unknown }).memorySearch).toMatchObject({ provider: "local", fallback: "none", @@ -905,8 +905,8 @@ describe("config strict validation", () => { const snap = await readConfigFileSnapshot(); expect(snap.valid).toBe(false); - expect(snap.issues.some((issue) => issue.path === "heartbeat")).toBe(true); - expect(snap.legacyIssues).toEqual([]); + expect(snap.issues.some((issue) => issue.message.includes('"heartbeat"'))).toBe(true); + expect(snap.legacyIssues.some((issue) => issue.path === "heartbeat")).toBe(true); expect((snap.sourceConfig as { heartbeat?: unknown }).heartbeat).toMatchObject({ every: "30m", model: "anthropic/claude-3-5-haiku-20241022", @@ -928,8 +928,8 @@ describe("config strict validation", () => { const snap = await readConfigFileSnapshot(); expect(snap.valid).toBe(false); - expect(snap.issues.some((issue) => issue.path === "heartbeat")).toBe(true); - expect(snap.legacyIssues).toEqual([]); + expect(snap.issues.some((issue) => issue.message.includes('"heartbeat"'))).toBe(true); + expect(snap.legacyIssues.some((issue) => issue.path === "heartbeat")).toBe(true); expect((snap.sourceConfig as { heartbeat?: unknown }).heartbeat).toMatchObject({ showOk: true, showAlerts: false, @@ -985,8 +985,11 @@ describe("config strict validation", () => { expect(snap.valid).toBe(false); expect(snap.issues.some((issue) => issue.path === "agents.defaults.sandbox")).toBe(true); - expect(snap.issues.some((issue) => issue.path === "agents.list")).toBe(true); - expect(snap.legacyIssues).toEqual([]); + expect(snap.issues.some((issue) => issue.path === "agents.list.0.sandbox")).toBe(true); + expect(snap.legacyIssues.some((issue) => issue.path === "agents.defaults.sandbox")).toBe( + true, + ); + expect(snap.legacyIssues.some((issue) => issue.path === "agents.list")).toBe(true); expect(snap.sourceConfig.agents?.defaults?.sandbox).toEqual({ perSession: true }); expect(snap.sourceConfig.agents?.list?.[0]?.sandbox).toEqual({ perSession: false }); }); @@ -1024,7 +1027,7 @@ describe("config strict validation", () => { const snap = await readConfigFileSnapshot(); expect(snap.valid).toBe(false); expect(snap.issues.some((issue) => issue.path === "gateway.bind")).toBe(true); - expect(snap.legacyIssues).toEqual([]); + expect(snap.legacyIssues.some((issue) => issue.path === "gateway.bind")).toBe(true); }); }); }); diff --git a/src/config/io.ts b/src/config/io.ts index b553f653e5b..c88b7bec079 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -1185,6 +1185,18 @@ async function finalizeReadConfigSnapshotInternalResult( return result; } +async function collectInvalidConfigLegacyIssues( + raw: unknown, + sourceRaw: unknown, +): Promise { + if (!raw || typeof raw !== "object") { + return []; + } + const { findDoctorLegacyConfigIssues } = + await import("../commands/doctor/shared/legacy-config-issues.js"); + return findDoctorLegacyConfigIssues(raw, sourceRaw); +} + export function createConfigIO( overrides: ConfigIoDeps & { pluginValidation?: "full" | "skip" } = {}, ) { @@ -1756,6 +1768,9 @@ export function createConfigIO( }), ); if (!validated.ok) { + const legacyIssues = await deps.measure("config.snapshot.read.legacy-issues", () => + collectInvalidConfigLegacyIssues(effectiveConfigRaw, effectiveParsed), + ); return await finalizeReadConfigSnapshotInternalResult(deps, { snapshot: createConfigFileSnapshot({ path: configPath, @@ -1768,7 +1783,7 @@ export function createConfigIO( hash: snapshotHash, issues: validated.issues, warnings: [...validated.warnings, ...envVarWarnings], - legacyIssues: [], + legacyIssues, }), }); } diff --git a/src/config/legacy.ts b/src/config/legacy.ts index 1fb703432d5..af1634e4c66 100644 --- a/src/config/legacy.ts +++ b/src/config/legacy.ts @@ -1,4 +1,3 @@ -import { collectChannelLegacyConfigRules } from "../channels/plugins/legacy-config.js"; import { LEGACY_CONFIG_RULES } from "./legacy.rules.js"; import type { LegacyConfigRule } from "./legacy.shared.js"; import type { LegacyConfigIssue } from "./types.js"; @@ -14,20 +13,6 @@ function getPathValue(root: Record, path: string[]): unknown { return cursor; } -function collectExplicitRuleOwnedChannelIds( - extraRules: readonly LegacyConfigRule[], -): ReadonlySet | undefined { - const channelIds = new Set(); - for (const rule of extraRules) { - const [first, second] = rule.path; - if (first !== "channels" || typeof second !== "string" || second === "defaults") { - continue; - } - channelIds.add(second); - } - return channelIds.size > 0 ? channelIds : undefined; -} - export function findLegacyConfigIssues( raw: unknown, sourceRaw?: unknown, @@ -41,12 +26,7 @@ export function findLegacyConfigIssues( const sourceRoot = sourceRaw && typeof sourceRaw === "object" ? (sourceRaw as Record) : root; const issues: LegacyConfigIssue[] = []; - const explicitRuleOwnedChannelIds = collectExplicitRuleOwnedChannelIds(extraRules); - for (const rule of [ - ...LEGACY_CONFIG_RULES, - ...collectChannelLegacyConfigRules(raw, touchedPaths, explicitRuleOwnedChannelIds), - ...extraRules, - ]) { + for (const rule of [...LEGACY_CONFIG_RULES, ...extraRules]) { const cursor = getPathValue(root, rule.path); if (cursor !== undefined && (!rule.match || rule.match(cursor, root))) { if (rule.requireSourceLiteral) { diff --git a/src/config/validation.legacy-rules-fast-path.test.ts b/src/config/validation.legacy-rules-fast-path.test.ts new file mode 100644 index 00000000000..7e2f616d955 --- /dev/null +++ b/src/config/validation.legacy-rules-fast-path.test.ts @@ -0,0 +1,110 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { LegacyConfigRule } from "./legacy.shared.js"; + +const { collectChannelLegacyConfigRulesMock, listPluginDoctorLegacyConfigRulesMock } = vi.hoisted( + () => ({ + collectChannelLegacyConfigRulesMock: vi.fn((): LegacyConfigRule[] => []), + listPluginDoctorLegacyConfigRulesMock: vi.fn((): LegacyConfigRule[] => []), + }), +); + +vi.mock("../channels/plugins/legacy-config.js", () => ({ + collectChannelLegacyConfigRules: collectChannelLegacyConfigRulesMock, +})); + +vi.mock("../plugins/doctor-contract-registry.js", () => ({ + listPluginDoctorLegacyConfigRules: listPluginDoctorLegacyConfigRulesMock, +})); + +import { validateConfigObjectRaw } from "./validation.js"; + +describe("config validation legacy rule loading", () => { + beforeEach(() => { + collectChannelLegacyConfigRulesMock.mockReset(); + collectChannelLegacyConfigRulesMock.mockReturnValue([]); + listPluginDoctorLegacyConfigRulesMock.mockReset(); + listPluginDoctorLegacyConfigRulesMock.mockReturnValue([]); + }); + + it("does not load channel or plugin doctor legacy rules for valid raw config", () => { + collectChannelLegacyConfigRulesMock.mockReturnValue([ + { + path: ["channels", "discord", "legacy"], + message: "legacy discord key", + }, + ]); + + const result = validateConfigObjectRaw({ + channels: { + discord: {}, + }, + }); + + expect(result.ok).toBe(true); + expect(collectChannelLegacyConfigRulesMock).not.toHaveBeenCalled(); + expect(listPluginDoctorLegacyConfigRulesMock).not.toHaveBeenCalled(); + }); + + it("does not load plugin doctor legacy rules for invalid raw config", () => { + listPluginDoctorLegacyConfigRulesMock.mockReturnValue([ + { + path: ["plugins", "entries", "demo", "legacy"], + message: "legacy demo key", + }, + ]); + + const result = validateConfigObjectRaw({ + plugins: { + entries: { + demo: { + legacy: true, + }, + }, + }, + }); + + expect(result.ok).toBe(false); + expect(collectChannelLegacyConfigRulesMock).not.toHaveBeenCalled(); + expect(listPluginDoctorLegacyConfigRulesMock).not.toHaveBeenCalled(); + }); + + it("skips enabled-only and empty-config plugin entries", () => { + const result = validateConfigObjectRaw({ + plugins: { + entries: { + anthropic: { + enabled: true, + }, + discord: { + config: {}, + enabled: true, + }, + }, + }, + }); + + expect(result.ok).toBe(true); + expect(collectChannelLegacyConfigRulesMock).not.toHaveBeenCalled(); + expect(listPluginDoctorLegacyConfigRulesMock).not.toHaveBeenCalled(); + }); + + it("does not use touched paths to load doctor rules during raw validation", () => { + const result = validateConfigObjectRaw( + { + plugins: { + entries: { + demo: {}, + other: {}, + }, + }, + }, + { + touchedPaths: [["plugins", "entries", "demo", "enabled"]], + }, + ); + + expect(result.ok).toBe(true); + expect(collectChannelLegacyConfigRulesMock).not.toHaveBeenCalled(); + expect(listPluginDoctorLegacyConfigRulesMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/config/validation.ts b/src/config/validation.ts index 1cb21901efa..a378111bd3e 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -8,11 +8,6 @@ import { resolveEffectivePluginActivationState, resolveMemorySlotDecision, } from "../plugins/config-state.js"; -import { - collectRelevantDoctorPluginIds, - collectRelevantDoctorPluginIdsForTouchedPaths, - listPluginDoctorLegacyConfigRules, -} from "../plugins/doctor-contract-registry.js"; import { loadInstalledPluginIndexInstallRecordsSync } from "../plugins/installed-plugin-index-record-reader.js"; import { resolveManifestCommandAliasOwnerInRegistry } from "../plugins/manifest-command-aliases.js"; import type { PluginManifestRegistry } from "../plugins/manifest-registry.js"; @@ -38,7 +33,6 @@ import { findDuplicateAgentDirs, formatDuplicateAgentDirError } from "./agent-di import { appendAllowedValuesHint, summarizeAllowedValues } from "./allowed-values.js"; import { GENERATED_BUNDLED_CHANNEL_CONFIG_METADATA } from "./bundled-channel-config-metadata.generated.js"; import { collectChannelSchemaMetadata } from "./channel-config-metadata.js"; -import { findLegacyConfigIssues } from "./legacy.js"; import { materializeRuntimeConfig } from "./materialize.js"; import type { OpenClawConfig, ConfigValidationIssue } from "./types.js"; import { coerceSecretRef } from "./types.secrets.js"; @@ -623,30 +617,6 @@ export function validateConfigObjectRaw( ): { ok: true; config: OpenClawConfig } | { ok: false; issues: ConfigValidationIssue[] } { const normalizedRaw = stripDeprecatedValidationKeys(raw); const policyIssues = collectUnsupportedSecretRefPolicyIssues(normalizedRaw); - const doctorPluginIds = opts?.touchedPaths - ? collectRelevantDoctorPluginIdsForTouchedPaths({ - raw: normalizedRaw, - touchedPaths: opts.touchedPaths, - }) - : collectRelevantDoctorPluginIds(normalizedRaw); - const extraLegacyRules = listPluginDoctorLegacyConfigRules({ - pluginIds: doctorPluginIds, - }); - const legacyIssues = findLegacyConfigIssues( - normalizedRaw, - opts?.sourceRaw ?? normalizedRaw, - extraLegacyRules, - opts?.touchedPaths, - ); - if (legacyIssues.length > 0) { - return { - ok: false, - issues: legacyIssues.map((iss) => ({ - path: iss.path, - message: iss.message, - })), - }; - } const validated = OpenClawSchema.safeParse(normalizedRaw); if (!validated.success) { const schemaIssues = validated.error.issues.map((issue) => mapZodIssueToConfigIssue(issue));