From fd6c9fc7f5f6a0bbe0a0dda6e0d25b4175e41a99 Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 27 Apr 2026 14:55:04 +0100 Subject: [PATCH] fix: reuse plugin registry during config validation --- CHANGELOG.md | 1 + src/config/io.ts | 110 +++++++++++------- .../validation.channel-metadata.test.ts | 51 +++++++- src/config/validation.ts | 77 ++++++------ src/gateway/server-startup-config.ts | 10 +- 5 files changed, 163 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a34823cd1d8..02fe278a512 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Docs: https://docs.openclaw.ai - Agents/reasoning: recover fully wrapped unclosed `` replies that would otherwise sanitize to empty text while keeping strict stripping for closed reasoning blocks and unclosed tails after visible text. Fixes #37696; supersedes #51915. Thanks @druide67 and @okuyam2y. - Control UI/Gateway: bind WebChat handshakes to their active socket and reject post-close server registrations, so aborted connects no longer leave zombie clients or misleading duplicate WebSocket connection logs. Fixes #72753. Thanks @LumenFromTheFuture. - Agents/fallback: split ambiguous provider failures into `empty_response`, `no_error_details`, and `unclassified`, and add flat fallback-step fields to structured fallback logs so primary-model failures stay visible when later fallbacks also fail. Fixes #71922; refs #71744. Thanks @andyk-ms and @nikolaykazakovvs-ux. +- Gateway/startup: reuse the plugin manifest registry inside config validation so restrictive plugin allowlists avoid a duplicate manifest pass during startup. Thanks @shakkernerd. - Gateway/startup: run plugin auto-enable from authored source config and skip disabled setup probes, avoiding runtime-default plugin allowlist writes and a second config snapshot read during startup. Thanks @shakkernerd. - Plugins/Windows: normalize Windows absolute paths before handing bundled plugin modules to Jiti, so Feishu/Lark message sending no longer fails with unsupported `c:` ESM loader URLs. Fixes #72783. Thanks @jackychen-png. - CLI/doctor: run bundled plugin runtime-dependency repairs through the async npm installer with spinner/line progress and heartbeat updates, so long `openclaw doctor --fix` installs no longer look hung in TTY or piped output. Fixes #72775. Thanks @dfpalhano. diff --git a/src/config/io.ts b/src/config/io.ts index 37937847c86..a86e96785d8 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -227,6 +227,7 @@ export type ReadConfigFileSnapshotForWriteResult = { }; export type ConfigWriteNotification = RuntimeConfigWriteNotification; +export type ConfigSnapshotReadMeasure = (name: string, run: () => T | Promise) => Promise; export class ConfigRuntimeRefreshError extends Error { constructor(message: string, options?: { cause?: unknown }) { @@ -903,6 +904,7 @@ export type ConfigIoDeps = { homedir?: () => string; configPath?: string; logger?: Pick; + measure?: ConfigSnapshotReadMeasure; }; function warnOnConfigMiskeys(raw: unknown, logger: Pick): void { @@ -960,6 +962,7 @@ function normalizeDeps(overrides: ConfigIoDeps = {}): Required { overrides.homedir ?? (() => resolveRequiredHomeDir(overrides.env ?? process.env, os.homedir)), configPath: overrides.configPath ?? "", logger: overrides.logger ?? console, + measure: overrides.measure ?? (async (_name, run) => await run()), }; } @@ -1637,11 +1640,15 @@ export function createConfigIO( let fallbackHash = hashConfigRaw(null); try { - const raw = deps.fs.readFileSync(configPath, "utf-8"); - const rawHash = hashConfigRaw(raw); + const raw = await deps.measure("config.snapshot.read.file", () => + deps.fs.readFileSync(configPath, "utf-8"), + ); + const rawHash = await deps.measure("config.snapshot.read.hash", () => hashConfigRaw(raw)); fallbackRaw = raw; fallbackHash = rawHash; - const parsedRes = parseConfigJson5(raw, deps.json5); + const parsedRes = await deps.measure("config.snapshot.read.parse", () => + parseConfigJson5(raw, deps.json5), + ); if (!parsedRes.ok) { return await finalizeReadConfigSnapshotInternalResult(deps, { snapshot: createConfigFileSnapshot({ @@ -1663,12 +1670,14 @@ export function createConfigIO( fallbackSourceConfig = coerceConfig(parsedRes.parsed); // Resolve $include directives - const recovered = await maybeRecoverSuspiciousConfigRead({ - deps, - configPath, - raw, - parsed: parsedRes.parsed, - }); + const recovered = await deps.measure("config.snapshot.read.recovery-check", () => + maybeRecoverSuspiciousConfigRead({ + deps, + configPath, + raw, + parsed: parsedRes.parsed, + }), + ); const effectiveRaw = recovered.raw; const effectiveParsed = recovered.parsed; const hash = hashConfigRaw(effectiveRaw); @@ -1679,7 +1688,9 @@ export function createConfigIO( let resolved: unknown; try { - resolved = resolveConfigIncludesForRead(effectiveParsed, configPath, deps); + resolved = await deps.measure("config.snapshot.read.includes", () => + resolveConfigIncludesForRead(effectiveParsed, configPath, deps), + ); } catch (err) { const message = err instanceof ConfigIncludeError @@ -1703,7 +1714,9 @@ export function createConfigIO( }); } - const readResolution = resolveConfigForRead(resolved, deps.env); + const readResolution = await deps.measure("config.snapshot.read.env", () => + resolveConfigForRead(resolved, deps.env), + ); // Convert missing env var references to config warnings instead of fatal errors. // This allows the gateway to start in degraded mode when non-critical config @@ -1714,13 +1727,16 @@ export function createConfigIO( })); const resolvedConfigRaw = readResolution.resolvedConfigRaw; - const legacyResolution = resolveLegacyConfigForRead(resolvedConfigRaw, effectiveParsed); - const installMigration = migrateAndStripShippedPluginInstallConfigRecords( - legacyResolution.effectiveConfigRaw, - { - persist: options.persistShippedPluginInstallMigration !== false, - rootConfigRaw: effectiveParsed, - }, + const legacyResolution = await deps.measure("config.snapshot.read.legacy", () => + resolveLegacyConfigForRead(resolvedConfigRaw, effectiveParsed), + ); + const installMigration = await deps.measure( + "config.snapshot.read.plugin-install-migration", + () => + migrateAndStripShippedPluginInstallConfigRecords(legacyResolution.effectiveConfigRaw, { + persist: options.persistShippedPluginInstallMigration !== false, + rootConfigRaw: effectiveParsed, + }), ); const effectiveConfigRaw = installMigration.config; const snapshotRaw = installMigration.persistedRootRaw ?? effectiveRaw; @@ -1729,10 +1745,12 @@ export function createConfigIO( ? hashConfigRaw(installMigration.persistedRootRaw) : hash; fallbackSourceConfig = coerceConfig(effectiveConfigRaw); - const validated = validateConfigObjectWithPlugins(effectiveConfigRaw, { - env: deps.env, - pluginValidation: overrides.pluginValidation, - }); + const validated = await deps.measure("config.snapshot.read.validate", () => + validateConfigObjectWithPlugins(effectiveConfigRaw, { + env: deps.env, + pluginValidation: overrides.pluginValidation, + }), + ); if (!validated.ok) { return await finalizeReadConfigSnapshotInternalResult(deps, { snapshot: createConfigFileSnapshot({ @@ -1752,25 +1770,29 @@ export function createConfigIO( } warnIfConfigFromFuture(validated.config, deps.logger); - const snapshotConfig = materializeRuntimeConfig(validated.config, "snapshot"); - return await finalizeReadConfigSnapshotInternalResult(deps, { - snapshot: createConfigFileSnapshot({ - path: configPath, - exists: true, - raw: snapshotRaw, - parsed: snapshotParsed, - // Use resolvedConfigRaw (after $include and ${ENV} substitution but BEFORE runtime defaults) - // for config set/unset operations (issue #6070) - sourceConfig: coerceConfig(effectiveConfigRaw), - valid: true, - runtimeConfig: snapshotConfig, - hash: snapshotHash, - issues: [], - warnings: [...validated.warnings, ...envVarWarnings], - legacyIssues: legacyResolution.sourceLegacyIssues, + const snapshotConfig = await deps.measure("config.snapshot.read.materialize", () => + materializeRuntimeConfig(validated.config, "snapshot"), + ); + return await deps.measure("config.snapshot.read.observe", () => + finalizeReadConfigSnapshotInternalResult(deps, { + snapshot: createConfigFileSnapshot({ + path: configPath, + exists: true, + raw: snapshotRaw, + parsed: snapshotParsed, + // Use resolvedConfigRaw (after $include and ${ENV} substitution but BEFORE runtime defaults) + // for config set/unset operations (issue #6070) + sourceConfig: coerceConfig(effectiveConfigRaw), + valid: true, + runtimeConfig: snapshotConfig, + hash: snapshotHash, + issues: [], + warnings: [...validated.warnings, ...envVarWarnings], + legacyIssues: legacyResolution.sourceLegacyIssues, + }), + envSnapshotForRestore: readResolution.envSnapshotForRestore, }), - envSnapshotForRestore: readResolution.envSnapshotForRestore, - }); + ); } catch (err) { const nodeErr = err as NodeJS.ErrnoException; let message: string; @@ -2304,8 +2326,12 @@ export async function readSourceConfigBestEffort(): Promise { return await createConfigIO().readSourceConfigBestEffort(); } -export async function readConfigFileSnapshot(): Promise { - return await createConfigIO().readConfigFileSnapshot(); +export async function readConfigFileSnapshot(options?: { + measure?: ConfigSnapshotReadMeasure; +}): Promise { + return await createConfigIO( + options?.measure ? { measure: options.measure } : {}, + ).readConfigFileSnapshot(); } export async function promoteConfigSnapshotToLastKnownGood( diff --git a/src/config/validation.channel-metadata.test.ts b/src/config/validation.channel-metadata.test.ts index 5adebcf8154..01f0f099760 100644 --- a/src/config/validation.channel-metadata.test.ts +++ b/src/config/validation.channel-metadata.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import type { PluginManifestRecord, PluginManifestRegistry } from "../plugins/manifest-registry.js"; import { validateConfigObjectRawWithPlugins, @@ -73,6 +73,27 @@ function createPluginConfigSchemaRegistry(): PluginManifestRegistry { }; } +function createCompatPluginConfigSchemaRegistry(): PluginManifestRegistry { + return { + diagnostics: [], + plugins: [ + createPluginManifestRecord({ + id: "opik", + configSchema: { + type: "object", + additionalProperties: true, + }, + }), + createPluginManifestRecord({ + id: "brave-search", + contracts: { + webSearchProviders: ["brave"], + }, + }), + ], + }; +} + function createPluginManifestRecord( overrides: Partial & Pick, ): PluginManifestRecord { @@ -95,6 +116,10 @@ vi.mock("../plugins/manifest-registry.js", () => ({ resolveManifestContractPluginIds: () => [], })); +vi.mock("../plugins/plugin-registry.js", () => ({ + loadPluginManifestRegistryForPluginRegistry: () => mockLoadPluginManifestRegistry(), +})); + vi.mock("../plugins/doctor-contract-registry.js", () => ({ collectRelevantDoctorPluginIds: () => [], listPluginDoctorLegacyConfigRules: () => [], @@ -119,6 +144,10 @@ function setupPluginSchemaWithRequiredDefault() { mockLoadPluginManifestRegistry.mockReturnValue(createPluginConfigSchemaRegistry()); } +beforeEach(() => { + mockLoadPluginManifestRegistry.mockClear(); +}); + describe("validateConfigObjectWithPlugins channel metadata (applyDefaults: true)", () => { it("applies bundled channel defaults from plugin-owned schema metadata", async () => { setupTelegramSchemaWithDefault(); @@ -186,3 +215,23 @@ describe("validateConfigObjectRawWithPlugins plugin config defaults", () => { } }); }); + +describe("validateConfigObjectWithPlugins bundled allowlist compatibility", () => { + it("reuses the manifest registry loaded for compatibility during plugin validation", () => { + mockLoadPluginManifestRegistry.mockReturnValue(createCompatPluginConfigSchemaRegistry()); + + const result = validateConfigObjectWithPlugins({ + plugins: { + allow: ["opik"], + entries: { + opik: { + enabled: true, + }, + }, + }, + }); + + expect(result.ok).toBe(true); + expect(mockLoadPluginManifestRegistry).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/config/validation.ts b/src/config/validation.ts index 2e0530da52b..b2c336f6fd1 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -776,6 +776,39 @@ function validateConfigObjectWithPluginsBase( let compatConfig: OpenClawConfig | null | undefined; let compatPluginIds: ReadonlySet | null = null; let compatPluginIdsResolved = false; + let registryDiagnosticsPushed = false; + + const pushRegistryDiagnostics = (registry: PluginManifestRegistry): void => { + if (registryDiagnosticsPushed) { + return; + } + registryDiagnosticsPushed = true; + for (const diag of registry.diagnostics) { + let path = diag.pluginId ? `plugins.entries.${diag.pluginId}` : "plugins"; + if (!diag.pluginId && diag.message.includes("plugin path not found")) { + path = "plugins.load.paths"; + } + const pluginLabel = diag.pluginId ? `plugin ${diag.pluginId}` : "plugin"; + const message = `${pluginLabel}: ${diag.message}`; + if (diag.level === "error") { + issues.push({ path, message }); + } else { + warnings.push({ path, message }); + } + } + }; + + const loadValidationRegistry = (): RegistryInfo => { + const workspaceDir = resolveAgentWorkspaceDir(config, resolveDefaultAgentId(config)); + const registry = loadPluginManifestRegistryForPluginRegistry({ + config, + workspaceDir: workspaceDir ?? undefined, + env: opts.env, + includeDisabled: true, + }); + registryInfo = { registry }; + return registryInfo; + }; const ensureCompatPluginIds = (): ReadonlySet => { if (compatPluginIdsResolved) { @@ -787,13 +820,7 @@ function validateConfigObjectWithPluginsBase( compatPluginIds = new Set(); return compatPluginIds; } - const workspaceDir = resolveAgentWorkspaceDir(config, resolveDefaultAgentId(config)); - const registry = loadPluginManifestRegistryForPluginRegistry({ - config, - workspaceDir: workspaceDir ?? undefined, - env: opts.env, - includeDisabled: true, - }); + const { registry } = registryInfo ?? loadValidationRegistry(); const overriddenBundledPluginIds = new Set( registry.diagnostics .filter((diag) => diag.message.includes("duplicate plugin id detected")) @@ -832,38 +859,10 @@ function validateConfigObjectWithPluginsBase( }; const ensureRegistry = (): RegistryInfo => { - if (registryInfo) { - return registryInfo; - } - - const effectiveConfig = ensureCompatConfig(); - const workspaceDir = resolveAgentWorkspaceDir( - effectiveConfig, - resolveDefaultAgentId(effectiveConfig), - ); - const registry = loadPluginManifestRegistryForPluginRegistry({ - config: effectiveConfig, - workspaceDir: workspaceDir ?? undefined, - env: opts.env, - includeDisabled: true, - }); - - for (const diag of registry.diagnostics) { - let path = diag.pluginId ? `plugins.entries.${diag.pluginId}` : "plugins"; - if (!diag.pluginId && diag.message.includes("plugin path not found")) { - path = "plugins.load.paths"; - } - const pluginLabel = diag.pluginId ? `plugin ${diag.pluginId}` : "plugin"; - const message = `${pluginLabel}: ${diag.message}`; - if (diag.level === "error") { - issues.push({ path, message }); - } else { - warnings.push({ path, message }); - } - } - - registryInfo = { registry }; - return registryInfo; + const info = registryInfo ?? loadValidationRegistry(); + ensureCompatConfig(); + pushRegistryDiagnostics(info.registry); + return info; }; const ensureKnownIds = (): Set => { diff --git a/src/gateway/server-startup-config.ts b/src/gateway/server-startup-config.ts index 3ddccc185f1..7a57ce75a7a 100644 --- a/src/gateway/server-startup-config.ts +++ b/src/gateway/server-startup-config.ts @@ -192,7 +192,9 @@ export async function loadGatewayStartupConfigSnapshot(params: { measure?: GatewayStartupConfigMeasure; }): Promise { const measure = params.measure ?? (async (_name, run) => await run()); - let configSnapshot = await measure("config.snapshot.read", () => readConfigFileSnapshot()); + let configSnapshot = await measure("config.snapshot.read", () => + readConfigFileSnapshot({ measure }), + ); let wroteConfig = false; let degradedStartupConfig = false; let degradedPluginConfig = false; @@ -241,7 +243,7 @@ export async function loadGatewayStartupConfigSnapshot(params: { `gateway: invalid config was restored from last-known-good backup: ${configSnapshot.path}`, ); configSnapshot = await measure("config.snapshot.recovery-read", () => - readConfigFileSnapshot(), + readConfigFileSnapshot({ measure }), ); if (configSnapshot.valid) { enqueueConfigRecoveryNotice({ @@ -258,7 +260,7 @@ export async function loadGatewayStartupConfigSnapshot(params: { `gateway: invalid config was repaired by stripping a non-JSON prefix: ${configSnapshot.path}`, ); configSnapshot = await measure("config.snapshot.prefix-recovery-read", () => - readConfigFileSnapshot(), + readConfigFileSnapshot({ measure }), ); } } @@ -287,7 +289,7 @@ export async function loadGatewayStartupConfigSnapshot(params: { }); wroteConfig = true; configSnapshot = await measure("config.snapshot.auto-enable-read", () => - readConfigFileSnapshot(), + readConfigFileSnapshot({ measure }), ); assertValidGatewayStartupConfigSnapshot(configSnapshot); params.log.info(