diff --git a/docs/platforms/mac/canvas.md b/docs/platforms/mac/canvas.md index 1e7c42365e6e..f7c7d1b8f9b8 100644 --- a/docs/platforms/mac/canvas.md +++ b/docs/platforms/mac/canvas.md @@ -89,6 +89,21 @@ widgets. Their renderer bundles continue to load from the Gateway's The macOS panel does not accept A2UI push/reset commands and does not automatically navigate to an A2UI page. +## Migrating documents from a custom root + +Run `openclaw doctor --fix` to move documents from the retired +`plugins.entries.canvas.config.host.root` (or the older `canvasHost.root`) into +the state directory's `canvas/documents` folder. An explicit plugin root takes +precedence over the older setting. Doctor removes the root setting only after no +legacy documents remain. If directory access or a document copy fails, doctor +warns and retains the source locator for retry. It may move the older setting +into the plugin config while preserving the path. A root that already points to +canonical storage, including through a symlink, needs no copy. + +Fix the reported permissions or target conflict, then rerun the command; do not +remove the root setting yourself. Hosted routes serve only the canonical folder, +so remaining legacy documents are unavailable until migration completes. + ## Related - [Show widget](/tools/show-widget) diff --git a/extensions/canvas/doctor-contract-api.ts b/extensions/canvas/doctor-contract-api.ts index 9e4d82cbeb4a..9e7845336191 100644 --- a/extensions/canvas/doctor-contract-api.ts +++ b/extensions/canvas/doctor-contract-api.ts @@ -2,15 +2,13 @@ import fs from "node:fs/promises"; import path from "node:path"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { resolvePluginConfigObject } from "openclaw/plugin-sdk/plugin-config-runtime"; import type { PluginDoctorStateMigration } from "openclaw/plugin-sdk/runtime-doctor-migrations"; import { pathExists } from "openclaw/plugin-sdk/security-runtime"; import { - asOptionalRecord as readRecord, - readStringValue as readString, -} from "openclaw/plugin-sdk/string-coerce-runtime"; -import { resolveUserPath } from "openclaw/plugin-sdk/text-utility-runtime"; -import { migrateCanvasHostConfig } from "./src/config-migration.js"; + listLegacyCanvasDocumentIds, + migrateCanvasHostConfig, + resolveLegacyCanvasDocumentsDir, +} from "./src/config-migration.js"; const RETIRED_CANVAS_HOST_CONFIG_PATH = ["plugins", "entries", "canvas", "config", "host"] as const; @@ -35,43 +33,16 @@ export function normalizeCompatibilityConfig({ cfg }: { cfg: OpenClawConfig }): return migrateCanvasHostConfig(cfg) ?? { config: cfg, changes: [] }; } -type StateMigrationParams = Parameters[0]; - -function resolveLegacyDocumentsDir(params: StateMigrationParams): string | null { - const pluginConfig = resolvePluginConfigObject(params.config, "canvas"); - const configuredRoot = readString(readRecord(pluginConfig?.host)?.root)?.trim(); - if (!configuredRoot) { - return null; - } - const legacyDir = path.join( - path.resolve(resolveUserPath(configuredRoot, params.env)), - "documents", - ); - const coreDir = path.resolve(params.stateDir, "canvas", "documents"); - return legacyDir === coreDir ? null : legacyDir; -} - -async function listDocumentIds(documentsDir: string): Promise { - try { - return (await fs.readdir(documentsDir, { withFileTypes: true })) - .filter((entry) => entry.isDirectory()) - .map((entry) => entry.name) - .toSorted(); - } catch { - return []; - } -} - export const stateMigrations: PluginDoctorStateMigration[] = [ { id: "canvas-custom-root-documents-to-core", label: "Canvas documents in a custom host root", async detectLegacyState(params) { - const legacyDir = resolveLegacyDocumentsDir(params); + const legacyDir = resolveLegacyCanvasDocumentsDir(params); if (!legacyDir) { return null; } - const documentIds = await listDocumentIds(legacyDir); + const documentIds = listLegacyCanvasDocumentIds(legacyDir); if (documentIds.length === 0) { return null; } @@ -85,11 +56,11 @@ export const stateMigrations: PluginDoctorStateMigration[] = [ async migrateLegacyState(params) { const changes: string[] = []; const warnings: string[] = []; - const legacyDir = resolveLegacyDocumentsDir(params); + const legacyDir = resolveLegacyCanvasDocumentsDir(params); if (!legacyDir) { return { changes, warnings }; } - const documentIds = await listDocumentIds(legacyDir); + const documentIds = listLegacyCanvasDocumentIds(legacyDir); if (documentIds.length === 0) { return { changes, warnings }; } @@ -121,7 +92,7 @@ export const stateMigrations: PluginDoctorStateMigration[] = [ migrated += 1; } catch (error) { warnings.push( - `Skipped Canvas document ${documentId}; core target may already exist: ${String(error)}`, + `Skipped Canvas document ${documentId}; core target may already exist: ${String(error)}. Keep plugins.entries.canvas.config.host.root, resolve the copy or target conflict, then rerun "openclaw doctor --fix".`, ); } finally { if (tempParent) { diff --git a/extensions/canvas/src/config-migration.test.ts b/extensions/canvas/src/config-migration.test.ts index e715de771c84..763a44125166 100644 --- a/extensions/canvas/src/config-migration.test.ts +++ b/extensions/canvas/src/config-migration.test.ts @@ -1,4 +1,7 @@ +import fs from "node:fs/promises"; +import path from "node:path"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { withTempHome } from "openclaw/plugin-sdk/test-env"; import { describe, expect, it } from "vitest"; import { migrateCanvasHostConfig } from "./config-migration.js"; @@ -74,4 +77,56 @@ describe("migrateCanvasHostConfig", () => { }), ).toBeNull(); }); + + it("retains an unresolved source root for a later resolved repair", () => { + const config: OpenClawConfig = { + plugins: { entries: { canvas: { config: { host: { root: "${CANVAS_MIGRATION_ROOT}" } } } } }, + }; + expect(migrateCanvasHostConfig(config)).toBeNull(); + }); + + it("moves an unresolved older root into the pending plugin setting", () => { + const root = "${CANVAS_MIGRATION_ROOT}"; + const result = migrateCanvasHostConfig({ canvasHost: { root } } as OpenClawConfig); + expect(result?.config).toEqual({ + plugins: { entries: { canvas: { config: { host: { root } } } } }, + }); + expect(migrateCanvasHostConfig(result!.config)).toBeNull(); + }); + + it.each(["inherited", "empty", "replaced"] as const)( + "preserves the shipped root precedence when the plugin root is %s", + async (scenario) => { + await withTempHome(async (home) => { + const legacyRoot = path.join(home, "legacy-canvas"); + const document = path.join(legacyRoot, "documents", "cv_existing"); + await fs.mkdir(document, { recursive: true }); + await fs.writeFile(path.join(document, "index.html"), "legacy"); + const root = scenario === "empty" ? "" : path.join(home, "unused-canvas"); + const result = migrateCanvasHostConfig({ + canvasHost: { enabled: false, root: legacyRoot }, + plugins: { + entries: { + canvas: { + config: { + host: { + enabled: true, + ...(scenario === "inherited" ? {} : { root }), + }, + }, + }, + }, + }, + } as OpenClawConfig); + expect(result?.config.plugins?.entries?.canvas?.config?.host).toEqual({ + enabled: true, + ...(scenario === "inherited" ? { root: legacyRoot } : {}), + }); + expect(result?.config).not.toHaveProperty("canvasHost"); + await expect(fs.readFile(path.join(document, "index.html"), "utf8")).resolves.toBe( + "legacy", + ); + }); + }, + ); }); diff --git a/extensions/canvas/src/config-migration.ts b/extensions/canvas/src/config-migration.ts index 9f7686a38ea7..8714953daa3b 100644 --- a/extensions/canvas/src/config-migration.ts +++ b/extensions/canvas/src/config-migration.ts @@ -1,24 +1,101 @@ /** Canvas config migration to the single surviving route-enable switch. */ +import fs from "node:fs"; +import path from "node:path"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { extractErrorCode } from "openclaw/plugin-sdk/error-runtime"; +import { resolvePluginConfigObject } from "openclaw/plugin-sdk/plugin-config-runtime"; +import { resolveStateDir } from "openclaw/plugin-sdk/state-paths"; import { asBoolean, asOptionalRecord as readRecord, + readStringValue as readString, } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { resolveUserPath } from "openclaw/plugin-sdk/text-utility-runtime"; const RETIRED_HOST_KEYS = ["root", "port", "liveReload"] as const; +function readLegacyCanvasRoot(config: OpenClawConfig): unknown { + // Stable releases merged canvasHost into plugin host config; plugin keys won. + const legacyHost = readRecord(readRecord(config)?.canvasHost); + const pluginHost = readRecord(resolvePluginConfigObject(config, "canvas")?.host); + return { ...legacyHost, ...pluginHost }.root; +} + +export function resolveLegacyCanvasDocumentsDir(params: { + config: OpenClawConfig; + env: NodeJS.ProcessEnv; + stateDir: string; +}): string | null { + const configuredRoot = readString(readLegacyCanvasRoot(params.config))?.trim(); + if (!configuredRoot) { + return null; + } + const legacyDir = path.join( + path.resolve(resolveUserPath(configuredRoot, params.env)), + "documents", + ); + const coreDir = path.resolve(params.stateDir, "canvas", "documents"); + if (legacyDir === coreDir) { + return null; + } + try { + if (fs.realpathSync(legacyDir) === fs.realpathSync(coreDir)) { + return null; + } + } catch { + // Missing or unreadable paths still need source inspection; only a proven + // canonical alias may bypass migration without examining its documents. + } + return legacyDir; +} + +export function listLegacyCanvasDocumentIds(documentsDir: string): string[] { + try { + return fs + .readdirSync(documentsDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .toSorted(); + } catch (error) { + if (extractErrorCode(error) === "ENOENT") { + return []; + } + throw new Error( + `Cannot read Canvas documents at ${documentsDir}: ${String(error)}. Keep plugins.entries.canvas.config.host.root, fix access, then rerun "openclaw doctor --fix".`, + { cause: error }, + ); + } +} + /** Removes retired file-host settings while preserving the route enablement choice. */ export function migrateCanvasHostConfig(config: OpenClawConfig): { config: OpenClawConfig; changes: string[]; } | null { const legacyHost = readRecord((config as { canvasHost?: unknown }).canvasHost); - const plugins = readRecord(config.plugins); - const entries = readRecord(plugins?.entries); - const canvasEntry = readRecord(entries?.canvas); - const canvasConfig = readRecord(canvasEntry?.config); + const canvasConfig = resolvePluginConfigObject(config, "canvas"); const existingHost = readRecord(canvasConfig?.host); - const retiredKeys = RETIRED_HOST_KEYS.filter((key) => Object.hasOwn(existingHost ?? {}, key)); + const configuredRoot = readLegacyCanvasRoot(config); + // An unresolved template is not evidence that its source directory is absent. + // Doctor's resolved config pass can retire it once the source is inspectable. + let retainRoot = readString(configuredRoot)?.includes("${") === true; + if (!retainRoot) { + // Normalization also runs before migration and through setup. Only a verified + // empty source may lose its retry locator; unreadable sources remain pending. + try { + const legacyDir = resolveLegacyCanvasDocumentsDir({ + config, + env: process.env, + stateDir: resolveStateDir(), + }); + retainRoot = legacyDir !== null && listLegacyCanvasDocumentIds(legacyDir).length > 0; + } catch { + retainRoot = true; + } + } + const retiredKeys = RETIRED_HOST_KEYS.filter( + (key) => Object.hasOwn(existingHost ?? {}, key) && !(key === "root" && retainRoot), + ); if (!legacyHost && retiredKeys.length === 0) { return null; } @@ -31,11 +108,14 @@ export function migrateCanvasHostConfig(config: OpenClawConfig): { const nextEntry = readRecord(nextEntries.canvas) ?? {}; const nextPluginConfig = readRecord(nextEntry.config) ?? {}; - if (existingHost || enabled !== undefined) { - if (enabled === undefined) { + if (existingHost || enabled !== undefined || retainRoot) { + if (enabled === undefined && !retainRoot) { delete nextPluginConfig.host; } else { - nextPluginConfig.host = { enabled }; + nextPluginConfig.host = { + ...(enabled !== undefined ? { enabled } : {}), + ...(retainRoot ? { root: configuredRoot } : {}), + }; } nextEntry.config = nextPluginConfig; nextEntries.canvas = nextEntry; @@ -46,9 +126,11 @@ export function migrateCanvasHostConfig(config: OpenClawConfig): { const changes: string[] = []; if (legacyHost) { changes.push( - enabled === undefined - ? "Removed retired canvasHost configuration." - : "Migrated canvasHost.enabled to plugins.entries.canvas.config.host.enabled.", + retainRoot + ? "Migrated canvasHost to plugins.entries.canvas.config.host; retained root for document migration retry." + : enabled === undefined + ? "Removed retired canvasHost configuration." + : "Migrated canvasHost.enabled to plugins.entries.canvas.config.host.enabled.", ); } if (retiredKeys.length > 0) { diff --git a/src/commands/doctor-config-flow.canvas-migration.test.ts b/src/commands/doctor-config-flow.canvas-migration.test.ts new file mode 100644 index 000000000000..34144392c77e --- /dev/null +++ b/src/commands/doctor-config-flow.canvas-migration.test.ts @@ -0,0 +1,159 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { readConfigFileSnapshot } from "../config/config.js"; +import { withTempHome, writeOpenClawConfig } from "../config/test-helpers.js"; +import { runInitialConfigWriteHealth } from "../flows/doctor-health-contribution-runners.config.js"; +import type { DoctorHealthFlowContext } from "../flows/doctor-health-contribution-types.js"; +import type { RuntimeEnv } from "../runtime.js"; +import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; +import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; +import { createDoctorPrompter, type DoctorOptions } from "./doctor-prompter.js"; + +const note = vi.hoisted(() => vi.fn<(message: string, title?: string) => void>()); +vi.mock("../../packages/terminal-core/src/note.js", () => ({ note })); + +async function repairConfig(configPath: string) { + const runtime: RuntimeEnv = { error: vi.fn(), exit: vi.fn(), log: vi.fn() }; + const options: DoctorOptions = { nonInteractive: true, repair: true }; + const prompter = createDoctorPrompter({ runtime, options }); + const configResult = await loadAndMaybeMigrateDoctorConfig({ + options, + confirm: (params) => prompter.confirm(params), + runtime, + prompter, + }); + const ctx: DoctorHealthFlowContext = { + runtime, + options, + prompter, + configResult, + cfg: configResult.cfg, + cfgForPersistence: structuredClone(configResult.cfg), + sourceConfigValid: configResult.sourceConfigValid ?? true, + configPath, + stateDirExistedAtStart: true, + ...(configResult.runWithPluginMetadataSnapshot + ? { runWithPluginMetadataSnapshot: configResult.runWithPluginMetadataSnapshot } + : {}), + }; + await runInitialConfigWriteHealth(ctx); + return JSON.parse(await fs.readFile(configPath, "utf8")); +} + +function writeCanvasConfig(home: string, root: string, legacy = false, port?: number) { + const host = { enabled: false, root, ...(port ? { port } : {}) }; + return writeOpenClawConfig(home, { + gateway: { mode: "local" }, + ...(legacy + ? { canvasHost: host } + : { plugins: { entries: { canvas: { enabled: true, config: { host } } } } }), + }); +} + +describe("Canvas document migration through doctor config persistence", () => { + afterEach(() => { + note.mockClear(); + closeOpenClawStateDatabaseForTest(); + }); + + // POSIX permissions exercise real read/copy failures; Windows and root ignore chmod(0). + it + .skipIf(process.platform === "win32" || process.getuid?.() === 0) + .each(["partial", "blind", "env-root", "legacy-root"] as const)( + "retains the root after a %s migration and retires it only after retry", + async (failure) => { + await withTempHome(async (home) => { + const customRoot = path.join(home, "custom-canvas"); + const documents = path.join(customRoot, "documents"); + const coreDocuments = path.join(home, ".openclaw", "canvas", "documents"); + for (const id of ["cv_first", "cv_retry"]) { + await fs.mkdir(path.join(documents, id), { recursive: true }); + await fs.writeFile(path.join(documents, id, "index.html"), id); + } + const configuredRoot = failure === "env-root" ? "${HOME}/custom-canvas" : customRoot; + const configPath = await writeCanvasConfig( + home, + configuredRoot, + failure === "legacy-root", + failure === "env-root" ? 18793 : undefined, + ); + const blockedPath = + failure === "blind" ? documents : path.join(documents, "cv_retry", "index.html"); + await fs.chmod(blockedPath, 0); + try { + const saved = await repairConfig(configPath); + expect + .soft(saved.plugins?.entries?.canvas?.config?.host?.root ?? saved.canvasHost?.root) + .toBe(configuredRoot); + if (failure === "env-root") { + // Prove a committed partial repair, not just a refused config write. + expect.soft(saved.plugins.entries.canvas.config.host).not.toHaveProperty("port"); + } + const warnings = note.mock.calls + .filter(([, title]) => title?.includes("warning")) + .map(([message]) => message) + .join("\n"); + expect.soft(warnings).toContain("Canvas"); + expect.soft(warnings).toContain("openclaw doctor --fix"); + if (failure === "blind") { + expect.soft(warnings).toContain("EACCES"); + } else { + expect.soft(warnings).toContain("cv_retry"); + await expect( + fs.readFile(path.join(coreDocuments, "cv_first", "index.html"), "utf8"), + ).resolves.toBe("cv_first"); + await expect(fs.access(path.join(coreDocuments, "cv_retry"))).rejects.toThrow(); + expect(await fs.readdir(coreDocuments)).toEqual(["cv_first"]); + } + } finally { + await fs.chmod(blockedPath, failure === "blind" ? 0o700 : 0o600); + } + + const saved = await repairConfig(configPath); + expect(saved.plugins?.entries?.canvas?.config?.host).toEqual({ enabled: false }); + for (const id of ["cv_first", "cv_retry"]) { + await expect( + fs.readFile(path.join(coreDocuments, id, "index.html"), "utf8"), + ).resolves.toBe(id); + await expect(fs.access(path.join(documents, id))).rejects.toThrow(); + } + expect((await readConfigFileSnapshot()).valid).toBe(true); + }); + }, + ); + + it.each(["complete", "empty", "absent", "canonical", "canonical-alias", "legacy-root"] as const)( + "retires a %s root without losing canonical documents", + async (scenario) => { + await withTempHome(async (home) => { + const coreRoot = path.join(home, ".openclaw", "canvas"); + const customRoot = scenario === "canonical" ? coreRoot : path.join(home, "custom-canvas"); + if (scenario === "canonical-alias") { + await fs.mkdir(coreRoot, { recursive: true }); + await fs.symlink(coreRoot, customRoot, "junction"); + } + const documents = path.join(customRoot, "documents"); + if (scenario !== "absent") { + await fs.mkdir(documents, { recursive: true }); + } + const hasDocument = scenario !== "empty" && scenario !== "absent"; + if (hasDocument) { + await fs.mkdir(path.join(documents, "cv_existing")); + await fs.writeFile(path.join(documents, "cv_existing", "index.html"), "existing"); + } + const saved = await repairConfig( + await writeCanvasConfig(home, customRoot, scenario === "legacy-root"), + ); + expect(saved.plugins.entries.canvas.config.host).toEqual({ enabled: false }); + expect(saved).not.toHaveProperty("canvasHost"); + expect((await readConfigFileSnapshot()).valid).toBe(true); + if (hasDocument) { + await expect( + fs.readFile(path.join(coreRoot, "documents", "cv_existing", "index.html"), "utf8"), + ).resolves.toBe("existing"); + } + }); + }, + ); +}); diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index 0cb772ef8d71..433fd3554691 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -1008,21 +1008,23 @@ vi.mock("../plugins/doctor-contract-registry.js", async () => { return changes.length > 0 ? { config: next, changes } : { config: cfg, changes: [] }; } + const collectRelevantDoctorPluginIds = (raw: unknown): string[] => { + const ids = new Set(); + const root = readNullableRecord(raw); + const channels = readNullableRecord(root?.channels); + for (const channelId of Object.keys(channels ?? {})) { + if (channelId !== "defaults") { + ids.add(channelId); + } + } + if (hasLegacyTalkFields(root?.talk)) { + ids.add("elevenlabs"); + } + return [...ids].toSorted(); + }; return { - collectRelevantDoctorPluginIds: (raw: unknown): string[] => { - const ids = new Set(); - const root = readNullableRecord(raw); - const channels = readNullableRecord(root?.channels); - for (const channelId of Object.keys(channels ?? {})) { - if (channelId !== "defaults") { - ids.add(channelId); - } - } - if (hasLegacyTalkFields(root?.talk)) { - ids.add("elevenlabs"); - } - return [...ids].toSorted(); - }, + collectRelevantDoctorPluginIds, + collectDoctorConfigRepairPluginIds: collectRelevantDoctorPluginIds, applyPluginDoctorCompatibilityMigrations: normalizeDiscordStreamingAliasesForTest, listPluginDoctorLegacyConfigRules: () => [ { @@ -1504,7 +1506,8 @@ vi.mock("./doctor-config-preflight.js", async () => { agentRosterIncludeOwned: injected?.agentRosterIncludeOwned === true, sourceConfigBeforeMigrations, config: effectiveConfig, - sourceConfig: effectiveConfig, + // The reader resolves source values but leaves legacy repairs to doctor. + sourceConfig: injectedEffectiveConfig, valid: legacyIssues.length === 0, warnings: [], legacyIssues, diff --git a/src/commands/doctor/shared/channel-legacy-config-migrate.test.ts b/src/commands/doctor/shared/channel-legacy-config-migrate.test.ts index 0ad46b1d2fac..5dc7035522bd 100644 --- a/src/commands/doctor/shared/channel-legacy-config-migrate.test.ts +++ b/src/commands/doctor/shared/channel-legacy-config-migrate.test.ts @@ -12,6 +12,8 @@ const loadBundledChannelDoctorContractApi = vi.hoisted(() => vi.fn()); const getBootstrapChannelPlugin = vi.hoisted(() => vi.fn()); vi.mock("../../../plugins/doctor-contract-registry.js", () => ({ + collectDoctorConfigRepairPluginIds: (...args: unknown[]) => + collectRelevantDoctorPluginIds(...args), applyPluginDoctorCompatibilityMigrations: (...args: unknown[]) => applyPluginDoctorCompatibilityMigrations(...args), collectRelevantDoctorPluginIds: (...args: unknown[]) => collectRelevantDoctorPluginIds(...args), diff --git a/src/commands/doctor/shared/channel-legacy-config-migrate.ts b/src/commands/doctor/shared/channel-legacy-config-migrate.ts index a680704b1687..e2265dde85f9 100644 --- a/src/commands/doctor/shared/channel-legacy-config-migrate.ts +++ b/src/commands/doctor/shared/channel-legacy-config-migrate.ts @@ -6,7 +6,7 @@ import { loadBundledChannelDoctorContractApi } from "../../../channels/plugins/d import type { OpenClawConfig } from "../../../config/types.js"; import { applyPluginDoctorCompatibilityMigrations, - collectRelevantDoctorPluginIds, + collectDoctorConfigRepairPluginIds, } from "../../../plugins/doctor-contract-registry.js"; import { listDoctorConfiguredChannelIds } from "./configured-channel-ids.js"; import { isRecord } from "./legacy-config-record-shared.js"; @@ -92,7 +92,7 @@ function collectPluginDoctorCompatibilityIds(params: { return [ ...new Set([ ...params.unresolvedChannelIds, - ...collectRelevantDoctorPluginIds(params.raw).filter( + ...collectDoctorConfigRepairPluginIds(params.raw).filter( (pluginId) => !unresolvedChannelIds.has(pluginId), ), ]), diff --git a/src/commands/doctor/shared/config-flow-steps.ts b/src/commands/doctor/shared/config-flow-steps.ts index cbce6402024c..e0aa7483220e 100644 --- a/src/commands/doctor/shared/config-flow-steps.ts +++ b/src/commands/doctor/shared/config-flow-steps.ts @@ -57,15 +57,13 @@ export function applyLegacyCompatibilityStep(params: { } } const hasAuthoredIncludes = containsAuthoredInclude(params.snapshot.parsed); - const migrationInput = hasAuthoredIncludes - ? params.snapshot.sourceConfig - : params.snapshot.parsed; + // State repairs must inspect resolved paths, not literal env templates. const { config: migrated, sourceConfig: migratedSource, changes, partiallyValid, - } = migrateLegacyConfig(migrationInput, { + } = migrateLegacyConfig(params.snapshot.sourceConfig, { authoredRaw: params.snapshot.parsed, resolvedRaw: params.snapshot.sourceConfig, }); diff --git a/src/commands/doctor/shared/invalid-plugin-config.test.ts b/src/commands/doctor/shared/invalid-plugin-config.test.ts index e87f5ddbc3e4..4bbf67874716 100644 --- a/src/commands/doctor/shared/invalid-plugin-config.test.ts +++ b/src/commands/doctor/shared/invalid-plugin-config.test.ts @@ -4,12 +4,17 @@ import type { OpenClawConfig } from "../../../config/types.openclaw.js"; const validationMocks = vi.hoisted(() => ({ validateConfigObjectWithPlugins: vi.fn(), + findDoctorLegacyConfigIssues: vi.fn((): Array<{ path: string; message: string }> => []), })); vi.mock("../../../config/validation.js", () => ({ validateConfigObjectWithPlugins: validationMocks.validateConfigObjectWithPlugins, })); +vi.mock("./legacy-config-issues.js", () => ({ + findDoctorLegacyConfigIssues: validationMocks.findDoctorLegacyConfigIssues, +})); + const [{ maybeRepairInvalidPluginConfig }, { migrateLegacyConfig }] = await Promise.all([ import("./invalid-plugin-config.js"), import("./legacy-config-migrate.js"), @@ -18,8 +23,32 @@ const [{ maybeRepairInvalidPluginConfig }, { migrateLegacyConfig }] = await Prom describe("doctor invalid plugin config repair", () => { beforeEach(() => { validationMocks.validateConfigObjectWithPlugins.mockReset(); + validationMocks.findDoctorLegacyConfigIssues.mockReset(); }); + it.each(["pending", "other"])( + "preserves only the plugin with a declared %s migration", + (migrationOwner) => { + validationMocks.validateConfigObjectWithPlugins.mockReturnValue({ + ok: false, + issues: [ + { path: "plugins.entries.pending.config", message: "invalid config: retired root" }, + ], + }); + validationMocks.findDoctorLegacyConfigIssues.mockReturnValue([ + { path: `plugins.entries.${migrationOwner}.config.root`, message: "Run doctor --fix" }, + ]); + const cfg: OpenClawConfig = { + plugins: { entries: { pending: { enabled: true, config: { root: "/legacy/documents" } } } }, + }; + const result = maybeRepairInvalidPluginConfig(cfg); + expect(result.config.plugins?.entries?.pending).toEqual( + migrationOwner === "pending" ? cfg.plugins?.entries?.pending : { enabled: false }, + ); + expect(result.changes).toHaveLength(migrationOwner === "pending" ? 0 : 1); + }, + ); + it("disables plugins and removes invalid config payloads", () => { validationMocks.validateConfigObjectWithPlugins.mockReturnValue({ ok: false, diff --git a/src/commands/doctor/shared/invalid-plugin-config.ts b/src/commands/doctor/shared/invalid-plugin-config.ts index 7e7d7166ec46..8f655ef44f17 100644 --- a/src/commands/doctor/shared/invalid-plugin-config.ts +++ b/src/commands/doctor/shared/invalid-plugin-config.ts @@ -3,35 +3,37 @@ import { asNullableRecord } from "@openclaw/normalization-core/record-coerce"; import { sanitizeForLog } from "../../../../packages/terminal-core/src/ansi.js"; import type { OpenClawConfig } from "../../../config/types.openclaw.js"; import { validateConfigObjectWithPlugins } from "../../../config/validation.js"; - -type InvalidPluginConfigHit = { - pluginId: string; - pathLabel: string; -}; +import { findDoctorLegacyConfigIssues } from "./legacy-config-issues.js"; const PLUGIN_CONFIG_ISSUE_RE = /^plugins\.entries\.([^.]+)\.config(?:\.|$)/; -function scanInvalidPluginConfig(cfg: OpenClawConfig): InvalidPluginConfigHit[] { +function scanInvalidPluginConfig(cfg: OpenClawConfig): Set { + const hits = new Set(); const validation = validateConfigObjectWithPlugins(cfg); if (validation.ok) { - return []; + return hits; } - const hits: InvalidPluginConfigHit[] = []; - const seen = new Set(); + const legacyIssues = findDoctorLegacyConfigIssues(cfg); for (const issue of validation.issues) { if (!issue.message.startsWith("invalid config:")) { continue; } const match = issue.path.match(PLUGIN_CONFIG_ISSUE_RE); const pluginId = match?.[1]; - if (!pluginId || seen.has(pluginId)) { + if (!pluginId || hits.has(pluginId)) { continue; } - seen.add(pluginId); - hits.push({ - pluginId, - pathLabel: `plugins.entries.${pluginId}.config`, - }); + // A pending owner migration may still need this invalid config as its source + // locator. Quarantine must not delete the only way to discover state on retry. + const configPath = `plugins.entries.${pluginId}.config`; + if ( + legacyIssues.some( + (legacy) => legacy.path === configPath || legacy.path.startsWith(`${configPath}.`), + ) + ) { + continue; + } + hits.add(pluginId); } return hits; } @@ -42,7 +44,7 @@ export function maybeRepairInvalidPluginConfig(cfg: OpenClawConfig): { changes: string[]; } { const hits = scanInvalidPluginConfig(cfg); - if (hits.length === 0) { + if (hits.size === 0) { return { config: cfg, changes: [] }; } @@ -53,8 +55,8 @@ export function maybeRepairInvalidPluginConfig(cfg: OpenClawConfig): { } const quarantined: string[] = []; - for (const hit of hits) { - const entry = asNullableRecord(entries[hit.pluginId]); + for (const pluginId of hits) { + const entry = asNullableRecord(entries[pluginId]); if (!entry) { continue; } @@ -62,7 +64,7 @@ export function maybeRepairInvalidPluginConfig(cfg: OpenClawConfig): { delete entry.config; } entry.enabled = false; - quarantined.push(hit.pluginId); + quarantined.push(pluginId); } if (quarantined.length === 0) { diff --git a/src/commands/doctor/shared/legacy-config-issues.ts b/src/commands/doctor/shared/legacy-config-issues.ts index 2774bbce45bf..e14345395126 100644 --- a/src/commands/doctor/shared/legacy-config-issues.ts +++ b/src/commands/doctor/shared/legacy-config-issues.ts @@ -9,8 +9,7 @@ import type { } from "../../../config/types.js"; import { withPluginMetadataSnapshotScope } from "../../../plugins/current-plugin-metadata-snapshot.js"; import { - collectRelevantDoctorPluginIds, - collectRelevantDoctorPluginIdsForTouchedPaths, + collectDoctorConfigRepairPluginIds, listPluginDoctorLegacyConfigRules, } from "../../../plugins/doctor-contract-registry.js"; import type { PluginMetadataSnapshot } from "../../../plugins/plugin-metadata-snapshot.types.js"; @@ -25,11 +24,9 @@ function collectPluginLegacyConfigRules( touchedPaths?: ReadonlyArray>, ): LegacyConfigRule[] { const channelIds = collectConfiguredChannelIds(raw); - const pluginIds = ( - touchedPaths - ? collectRelevantDoctorPluginIdsForTouchedPaths({ raw, touchedPaths }) - : collectRelevantDoctorPluginIds(raw) - ).filter((pluginId) => !channelIds.has(pluginId)); + const pluginIds = collectDoctorConfigRepairPluginIds(raw, touchedPaths).filter( + (pluginId) => !channelIds.has(pluginId), + ); if (pluginIds.length === 0) { return []; } diff --git a/src/commands/doctor/shared/legacy-config-state-migration-input.ts b/src/commands/doctor/shared/legacy-config-state-migration-input.ts index 31de1dbd878a..f67a303cfd57 100644 --- a/src/commands/doctor/shared/legacy-config-state-migration-input.ts +++ b/src/commands/doctor/shared/legacy-config-state-migration-input.ts @@ -24,10 +24,9 @@ export function resolveStateMigrationConfigInput(params: { return null; } const migrated = migrateLegacyConfig(migrationSource); - if (!migrated.config) { - return null; - } - if (migrated.partiallyValid) { + // Plugin config repair may retain a legacy locator until its state migration + // completes. No config mutation must not prevent that owner from retrying. + if (!migrated.config || migrated.partiallyValid) { return { pluginDoctorConfig: (pluginDoctorConfig ?? migrationSource) as OpenClawConfig, }; diff --git a/src/config/validation.channel-metadata.test.ts b/src/config/validation.channel-metadata.test.ts index 2cc9b2a3e850..6c0767897129 100644 --- a/src/config/validation.channel-metadata.test.ts +++ b/src/config/validation.channel-metadata.test.ts @@ -260,6 +260,7 @@ vi.mock("../plugins/plugin-metadata-snapshot.js", () => ({ })); vi.mock("../plugins/doctor-contract-registry.js", () => ({ + collectDoctorConfigRepairPluginIds: () => [], collectRelevantDoctorPluginIds: () => [], listPluginDoctorLegacyConfigRules: () => [], applyPluginDoctorCompatibilityMigrations: () => ({ next: null, changes: [] }), diff --git a/src/config/validation.policy.test.ts b/src/config/validation.policy.test.ts index 9dc9240b884b..9f6ce7edc4be 100644 --- a/src/config/validation.policy.test.ts +++ b/src/config/validation.policy.test.ts @@ -12,6 +12,7 @@ vi.mock("../channels/plugins/legacy-config.js", () => ({ })); vi.mock("../plugins/doctor-contract-registry.js", () => ({ + collectDoctorConfigRepairPluginIds: () => [], collectRelevantDoctorPluginIds: () => [], listPluginDoctorLegacyConfigRules: () => [], })); diff --git a/src/plugins/config-contract-matches.ts b/src/plugins/config-contract-matches.ts index bf38dcc2e37b..bcad3994404b 100644 --- a/src/plugins/config-contract-matches.ts +++ b/src/plugins/config-contract-matches.ts @@ -24,6 +24,29 @@ function normalizePathPattern(pathPattern: string): string[] { return normalizeStringEntries(pathPattern.split(".")); } +/** Match declared migration sources without widening a scoped config edit. */ +export function hasPluginConfigMigrationSource(params: { + root: unknown; + pathPatterns?: readonly string[]; + touchedPaths?: ReadonlyArray>; +}): boolean { + return ( + params.pathPatterns?.some((pathPattern) => { + const pattern = normalizePathPattern(pathPattern); + const touched = + !params.touchedPaths || + params.touchedPaths.some((parts) => + pattern + .slice(0, parts.length) + .every((segment, index) => segment === "*" || segment === parts[index]), + ); + return ( + touched && collectPluginConfigContractMatches({ root: params.root, pathPattern }).length > 0 + ); + }) ?? false + ); +} + function parseCanonicalArrayIndex(segment: string, length: number): number | null { const index = parseConfigPathArrayIndex(segment); return index !== undefined && index < length ? index : null; diff --git a/src/plugins/doctor-contract-registry.test.ts b/src/plugins/doctor-contract-registry.test.ts index 63e05419c9a6..253ece8fd3c3 100644 --- a/src/plugins/doctor-contract-registry.test.ts +++ b/src/plugins/doctor-contract-registry.test.ts @@ -28,7 +28,7 @@ vi.mock("../logging/subsystem.js", async (importOriginal) => { let applyPluginDoctorCompatibilityMigrations: typeof import("./doctor-contract-registry.js").applyPluginDoctorCompatibilityMigrations; let clearPluginDoctorContractRegistryCache: typeof import("./doctor-contract-registry.test-fixtures.js").clearPluginDoctorContractRegistryCache; let collectRelevantDoctorPluginIds: typeof import("./doctor-contract-registry.js").collectRelevantDoctorPluginIds; -let collectRelevantDoctorPluginIdsForTouchedPaths: typeof import("./doctor-contract-registry.js").collectRelevantDoctorPluginIdsForTouchedPaths; +let collectDoctorConfigRepairPluginIds: typeof import("./doctor-contract-registry.js").collectDoctorConfigRepairPluginIds; let listPluginDoctorLegacyConfigRules: typeof import("./doctor-contract-registry.js").listPluginDoctorLegacyConfigRules; let listPluginDoctorSessionRouteStateOwners: typeof import("./doctor-contract-registry.js").listPluginDoctorSessionRouteStateOwners; let listPluginDoctorSessionStoreAgentIds: typeof import("./doctor-contract-registry.js").listPluginDoctorSessionStoreAgentIds; @@ -56,12 +56,13 @@ afterEach(() => { describe("doctor-contract-registry module loader", () => { beforeEach(async () => { resetRegistryJitiMocks(); + mocks.loadPluginManifestRegistry.mockReturnValue({ plugins: [], diagnostics: [] }); doctorContractWarnMock.mockReset(); vi.resetModules(); ({ applyPluginDoctorCompatibilityMigrations, collectRelevantDoctorPluginIds, - collectRelevantDoctorPluginIdsForTouchedPaths, + collectDoctorConfigRepairPluginIds, listPluginDoctorLegacyConfigRules, listPluginDoctorSessionRouteStateOwners, listPluginDoctorSessionStoreAgentIds, @@ -555,6 +556,63 @@ describe("doctor-contract-registry module loader", () => { expect(mocks.loadPluginManifestRegistry).toHaveBeenCalledTimes(2); }); + it.each([ + { name: "full scan", touchedPaths: undefined, configRepair: true, expected: true }, + { name: "parent edit", touchedPaths: [["legacyRoots"]], configRepair: true, expected: true }, + { + name: "dotted owner edit", + touchedPaths: [["legacyRoots", "store.with.dots", "root"]], + configRepair: true, + expected: true, + }, + { + name: "unrelated edit", + touchedPaths: [["gateway", "port"]], + configRepair: true, + expected: false, + }, + { name: "empty edit", touchedPaths: [], configRepair: true, expected: false }, + { name: "undeclared repair", touchedPaths: undefined, configRepair: false, expected: false }, + ])( + "discovers declared config migration sources without plugin entries: $name", + async ({ touchedPaths, configRepair, expected }) => { + const pluginRoot = makeTempDir(); + fs.writeFileSync(path.join(pluginRoot, "doctor-contract-api.ts"), "export {};\n", "utf-8"); + const rule = { + path: ["legacyRoots", "store.with.dots", "root"], + message: "Migrate legacy root", + }; + mocks.createJiti.mockImplementation(() => () => ({ + legacyConfigRules: [rule], + resolveSessionStoreAgentIds: () => ["unexpected-owner"], + })); + mocks.loadPluginManifestRegistry.mockReturnValue({ + plugins: [ + { + id: "root-owner", + rootDir: pluginRoot, + channels: [], + providers: [], + doctorContract: { configRepair, resolveSessionStoreAgentIds: true }, + configContracts: { compatibilityMigrationPaths: ["legacyRoots.*.root"] }, + }, + ], + diagnostics: [], + }); + const raw = { legacyRoots: { "store.with.dots": { root: "/legacy/documents" } } }; + const { findDoctorLegacyConfigIssues } = + await import("../commands/doctor/shared/legacy-config-issues.js"); + expect(findDoctorLegacyConfigIssues(raw, raw, touchedPaths)).toEqual( + expected ? [{ path: rule.path.join("."), message: rule.message }] : [], + ); + expect(mocks.createJiti).toHaveBeenCalledTimes(expected ? 1 : 0); + // Config migration declarations must not select new session-store owners. + const pluginIds = collectRelevantDoctorPluginIds(raw); + expect(pluginIds).toEqual([]); + expect(listPluginDoctorSessionStoreAgentIds({ pluginIds })).toEqual([]); + }, + ); + it("collects model provider ids for doctor compatibility migrations", () => { expect( collectRelevantDoctorPluginIds({ @@ -720,14 +778,9 @@ describe("doctor-contract-registry module loader", () => { expect(collectRelevantDoctorPluginIds(raw)).toEqual(["discord", "openai"]); expect( - collectRelevantDoctorPluginIdsForTouchedPaths({ - raw, - touchedPaths: [["channels", "modelByChannel", "discord", "guild"]], - }), + collectDoctorConfigRepairPluginIds(raw, [["channels", "modelByChannel", "discord", "guild"]]), ).toStrictEqual(["openai"]); - expect( - collectRelevantDoctorPluginIdsForTouchedPaths({ raw, touchedPaths: [["channels"]] }), - ).toEqual(["discord", "openai"]); + expect(collectDoctorConfigRepairPluginIds(raw, [["channels"]])).toEqual(["discord", "openai"]); }); it("collects provider ids from media model entries", () => { @@ -747,10 +800,7 @@ describe("doctor-contract-registry module loader", () => { expect(collectRelevantDoctorPluginIds(raw)).toEqual(["gemini", "openai", "xai"]); expect( - collectRelevantDoctorPluginIdsForTouchedPaths({ - raw, - touchedPaths: [["tools", "media", "models", "2", "model"]], - }), + collectDoctorConfigRepairPluginIds(raw, [["tools", "media", "models", "2", "model"]]), ).toEqual(["gemini", "openai", "xai"]); }); @@ -852,8 +902,8 @@ describe("doctor-contract-registry module loader", () => { it("narrows touched-path doctor ids for scoped dry-run validation", () => { expect( - collectRelevantDoctorPluginIdsForTouchedPaths({ - raw: { + collectDoctorConfigRepairPluginIds( + { channels: { discord: {}, telegram: {}, @@ -872,20 +922,20 @@ describe("doctor-contract-registry module loader", () => { voiceId: "legacy-voice", }, }, - touchedPaths: [ + [ ["channels", "discord", "token"], ["plugins", "entries", "memory-wiki", "enabled"], ["models", "providers", "ollama-cloud", "baseUrl"], ["talk", "voiceId"], ], - }), + ), ).toEqual(["discord", "elevenlabs", "memory-wiki", "ollama-cloud"]); }); it("keeps all configured model and policy providers active during touched scans", () => { expect( - collectRelevantDoctorPluginIdsForTouchedPaths({ - raw: { + collectDoctorConfigRepairPluginIds( + { agents: { defaults: { model: { primary: "agent-primary/model", fallbacks: ["agent-fallback/model"] }, @@ -901,13 +951,13 @@ describe("doctor-contract-registry module loader", () => { discord: { voice: { model: "untouched-voice/model" } }, }, }, - touchedPaths: [ + [ ["agents", "defaults", "model"], ["agents", "entries", "worker", "modelPolicy", "allow", "0"], ["hooks", "gmail", "model"], ["channels", "modelByChannel", "slack", "room"], ], - }), + ), ).toEqual([ "agent-fallback", "agent-primary", @@ -921,20 +971,20 @@ describe("doctor-contract-registry module loader", () => { it("does not infer touched-path ownership from dotted configured ids", () => { expect( - collectRelevantDoctorPluginIdsForTouchedPaths({ - raw: { + collectDoctorConfigRepairPluginIds( + { agents: { entries: { "worker.blue": { model: "provider.with.dots/model" } } }, plugins: { entries: { other: {} } }, }, - touchedPaths: [["plugins", "entries", "other", "enabled"]], - }), + [["plugins", "entries", "other", "enabled"]], + ), ).toEqual(["other", "provider.with.dots"]); }); it("falls back to the full doctor-id set when touched paths are too broad", () => { expect( - collectRelevantDoctorPluginIdsForTouchedPaths({ - raw: { + collectDoctorConfigRepairPluginIds( + { channels: { discord: {}, telegram: {}, @@ -945,8 +995,8 @@ describe("doctor-contract-registry module loader", () => { }, }, }, - touchedPaths: [["channels"]], - }), + [["channels"]], + ), ).toEqual(["discord", "memory-wiki", "telegram"]); }); }); diff --git a/src/plugins/doctor-contract-registry.ts b/src/plugins/doctor-contract-registry.ts index 874da2d81c15..f09f8745cffd 100644 --- a/src/plugins/doctor-contract-registry.ts +++ b/src/plugins/doctor-contract-registry.ts @@ -13,6 +13,7 @@ import { createSubsystemLogger } from "../logging/subsystem.js"; import type { BundledChannelSetupEntryContract } from "../plugin-sdk/channel-entry-contract.js"; import type { BundledChannelLegacyStateMigrationDetector } from "../plugin-sdk/channel-entry-contract.types.js"; import { definePluginDoctorMigrationFromPlans } from "../plugin-sdk/doctor-migration-plan-adapter.js"; +import { hasPluginConfigMigrationSource } from "./config-contract-matches.js"; import { normalizePluginsConfig } from "./config-state.js"; import { resolvePluginDoctorContractArtifactPath } from "./doctor-contract-artifact.js"; import { @@ -181,7 +182,7 @@ export function collectRelevantDoctorPluginIds(raw: unknown): string[] { return [...ids].toSorted(); } -export function collectRelevantDoctorPluginIdsForTouchedPaths(params: { +function collectRelevantDoctorPluginIdsForTouchedPaths(params: { raw: unknown; touchedPaths: ReadonlyArray>; }): string[] { @@ -230,6 +231,38 @@ export function collectRelevantDoctorPluginIdsForTouchedPaths(params: { return [...ids].toSorted(); } +/** Include manifest-owned legacy roots for config repair, never session ownership. */ +export function collectDoctorConfigRepairPluginIds( + raw: unknown, + touchedPaths?: ReadonlyArray>, +): string[] { + const config = asNullableRecord(raw); + if (!config) { + return []; + } + const ids = new Set( + touchedPaths + ? collectRelevantDoctorPluginIdsForTouchedPaths({ raw, touchedPaths }) + : collectRelevantDoctorPluginIds(raw), + ); + const registry = loadPluginManifestRegistryForPluginRegistry({ + config, + includeDisabled: true, + }); + for (const plugin of registry.plugins) { + if ( + hasPluginConfigMigrationSource({ + root: raw, + pathPatterns: plugin.configContracts?.compatibilityMigrationPaths, + touchedPaths, + }) + ) { + ids.add(plugin.id); + } + } + return [...ids].toSorted(); +} + function loadPluginDoctorContractEntry( record: PluginManifestRegistryRecord, ): PluginDoctorContractEntry | null { diff --git a/src/plugins/setup-registry.ts b/src/plugins/setup-registry.ts index f96b2af3f8db..9cc8696ce755 100644 --- a/src/plugins/setup-registry.ts +++ b/src/plugins/setup-registry.ts @@ -10,7 +10,7 @@ import { import type { OpenClawConfig } from "../config/types.openclaw.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { buildPluginApi, createUnavailableRuntime } from "./api-builder.js"; -import { collectPluginConfigContractMatches } from "./config-contracts.js"; +import { hasPluginConfigMigrationSource } from "./config-contract-matches.js"; import { getCurrentPluginMetadataSnapshotState } from "./current-plugin-metadata-state.js"; import type { PluginManifestRecord, PluginManifestRegistry } from "./manifest-registry.js"; import { createPluginCacheKey, PluginLruCache } from "./plugin-cache-primitives.js"; @@ -192,18 +192,11 @@ function resolveRelevantSetupMigrationPluginIds(params: { env: params.env, }); for (const plugin of registry.plugins) { - const paths = plugin.configContracts?.compatibilityMigrationPaths; - if (!paths?.length) { - continue; - } if ( - paths.some( - (pathPattern) => - collectPluginConfigContractMatches({ - root: params.config, - pathPattern, - }).length > 0, - ) + hasPluginConfigMigrationSource({ + root: params.config, + pathPatterns: plugin.configContracts?.compatibilityMigrationPaths, + }) ) { ids.add(plugin.id); }